Mailing List Archive

possible problem with prefix_bit
Hi all,

I do not know if this list is still valid but anyway I will express my anxiety for a code snippet:
Inside lib/table.c there is function set_link:
static void
set_link (struct route_node *node, struct route_node *new)
{
  unsigned int bit = prefix_bit (&new->p.u.prefix, node->p.prefixlen);

  node->link[bit] = new;
  new->parent = node;
}
that calls function prefix_bit:
unsigned int
prefix_bit (const u_char *prefix, const u_char prefixlen)
{
  unsigned int offset = prefixlen / 8;
  unsigned int shift  = 7 - (prefixlen % 8);
 
  return (prefix[offset] >> shift) & 1;
}

I suppose that prefixlen could also be equal to 32 for an IPV4 address that could result to a buffer overrun insideprefix_bit. Am I right?
Best regards,Kostas Sotiropoulos
Re: possible problem with prefix_bit [ In reply to ]
On Mon, Jul 13, 2020 at 12:00:02PM +0100, quagga-dev-request@lists.quagga.net wrote:
> Message: 1
> Date: Sun, 12 Jul 2020 21:22:15 +0000 (UTC)
> From: Kostas Sotiropoulos <kosotiro@yahoo.gr>
> To: "quagga-dev@lists.quagga.net" <quagga-dev@lists.quagga.net>
> Subject: [quagga-dev 16762] possible problem with prefix_bit
> Message-ID: <1148511041.807225.1594588935714@mail.yahoo.com>
> Content-Type: text/plain; charset="utf-8"
>
> Hi all,
>
> I do not know if this list is still valid but anyway I will express my anxiety for a code snippet:
> Inside lib/table.c there is function set_link:
> static void
> set_link (struct route_node *node, struct route_node *new)
> {
> ? unsigned int bit = prefix_bit (&new->p.u.prefix, node->p.prefixlen);
>
> ? node->link[bit] = new;
> ? new->parent = node;
> }
> that calls function prefix_bit:
> unsigned int
> prefix_bit (const u_char *prefix, const u_char prefixlen)
> {
> ? unsigned int offset = prefixlen / 8;
> ? unsigned int shift? = 7 - (prefixlen % 8);
> ?
> ? return (prefix[offset] >> shift) & 1;
> }
>
> I suppose that prefixlen could also be equal to 32 for an IPV4 address that could result to a buffer overrun insideprefix_bit. Am I right?
> Best regards,Kostas Sotiropoulos


[ Disclaimer: I'm not a developer ]

Hi,

note that struct prefix is at least 8 bytes long, and AFAICT is usually
embedded within a larger struct (e.g. struct route_node) with more
components following after struct prefix.

With 8 bytes size, prefixlen==32 for an IPv4 address will not read from
outside struct prefix (offset==4).

With prefixlen==128 for an IPv6 address it might read 1 byte after struct
prefix (offset==8), but still from valid memory.

Matthias Ferdinand
_______________________________________________
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev
Re: possible problem with prefix_bit [ In reply to ]
Hi,

@Matthias: As you correctly said &new->p.u.prefix points to a memory of 8
bytes, since union size is equal
to the size of maximum field and if someone writes to one field then it will
occupy this memory.
But struct in_addr is of 4 bytes long for an IPV4 address and we will
actually take decisions
inside prefix_bit from a position that actually is at offset 4, which will
probably will always
be "0". About IPV6 I do not understand why you say that the 1 extra byte we
will read will be still
in a valid memory area since then size of union u will be equal to 16 bytes
and we will read from
offset 16 when prefixlen is equal to 128 bits.

struct prefix
{
u_char family;
u_char prefixlen;
union
{
u_char prefix;
struct in_addr prefix4;
#ifdef HAVE_IPV6
struct in6_addr prefix6;
#endif /* HAVE_IPV6 */
struct
{
struct in_addr id;
struct in_addr adv_router;
} lp;
struct ethaddr prefix_eth; /* AF_ETHERNET */
u_char val[8];
uintptr_t ptr;
} u __attribute__ ((aligned (8)));
}

My point here is that in prefix_bit I would expect the offset to be equal
sizeof(struct in_addr) - 1
or sizeof (struct in6_addr) - 1 in order to be absolutely correct. It seems
that check_bit is a legacy
from zebra software. Please correct me if I miss anything here.

@ Dariusz: I am not so sure that we are so secure of not calling the
function with either prefixlen = 32 or 128.

Kostas Sotiropoulos

-----?????? ??????-----
???: quagga-dev-bounces@lists.quagga.net
[mailto:quagga-dev-bounces@lists.quagga.net] ?? ?????? ???
quagga-dev-request@lists.quagga.net
????????: Tuesday, July 14, 2020 2:00 PM
????: quagga-dev@lists.quagga.net
????: Quagga-dev Digest, Vol 188, Issue 2

Send Quagga-dev mailing list submissions to
quagga-dev@lists.quagga.net

To subscribe or unsubscribe via the World Wide Web, visit
https://lists.quagga.net/mailman/listinfo/quagga-dev

You can reach the person managing the list at
quagga-dev-owner@lists.quagga.net

When replying, please edit your Subject line so it is more specific
than "Re: Contents of Quagga-dev digest..."


Today's Topics:

1. [quagga-dev 16763] Re: possible problem with prefix_bit
(Matthias Ferdinand)
2. [quagga-dev 16764] ODP: Re: possible problem with prefix_bit
(Dariusz Grzeg?rski)


----------------------------------------------------------------------

Message: 1
Date: Mon, 13 Jul 2020 14:29:50 +0200
From: Matthias Ferdinand <mf@14v.de>
To: quagga-dev@lists.quagga.net
Subject: [quagga-dev 16763] Re: possible problem with prefix_bit
Message-ID: <20200713122950.GE23382@xoff>
Content-Type: text/plain; charset=utf-8

On Mon, Jul 13, 2020 at 12:00:02PM +0100,
quagga-dev-request@lists.quagga.net wrote:
> Message: 1
> Date: Sun, 12 Jul 2020 21:22:15 +0000 (UTC)
> From: Kostas Sotiropoulos <kosotiro@yahoo.gr>
> To: "quagga-dev@lists.quagga.net" <quagga-dev@lists.quagga.net>
> Subject: [quagga-dev 16762] possible problem with prefix_bit
> Message-ID: <1148511041.807225.1594588935714@mail.yahoo.com>
> Content-Type: text/plain; charset="utf-8"
>
> Hi all,
>
> I do not know if this list is still valid but anyway I will express my
anxiety for a code snippet:
> Inside lib/table.c there is function set_link:
> static void
> set_link (struct route_node *node, struct route_node *new)
> {
> ? unsigned int bit = prefix_bit (&new->p.u.prefix, node->p.prefixlen);
>
> ? node->link[bit] = new;
> ? new->parent = node;
> }
> that calls function prefix_bit:
> unsigned int
> prefix_bit (const u_char *prefix, const u_char prefixlen)
> {
> ? unsigned int offset = prefixlen / 8;
> ? unsigned int shift? = 7 - (prefixlen % 8);
> ?
> ? return (prefix[offset] >> shift) & 1;
> }
>
> I suppose that prefixlen could also be equal to 32 for an IPV4 address
that could result to a buffer overrun insideprefix_bit. Am I right?
> Best regards,Kostas Sotiropoulos


[ Disclaimer: I'm not a developer ]

Hi,

note that struct prefix is at least 8 bytes long, and AFAICT is usually
embedded within a larger struct (e.g. struct route_node) with more
components following after struct prefix.

With 8 bytes size, prefixlen==32 for an IPv4 address will not read from
outside struct prefix (offset==4).

With prefixlen==128 for an IPv6 address it might read 1 byte after struct
prefix (offset==8), but still from valid memory.

Matthias Ferdinand


------------------------------

Message: 2
Date: Mon, 13 Jul 2020 13:34:17 +0000
From: Dariusz Grzeg?rski <dariusz.grzegorski@transbit.com.pl>
To: "quagga-dev@lists.quagga.net" <quagga-dev@lists.quagga.net>
Subject: [quagga-dev 16764] ODP: Re: possible problem with prefix_bit
Message-ID:

<AM0PR04MB43371E652506CC4518FBDCF6CF600@AM0PR04MB4337.eurprd04.prod.outlook.

Content-Type: text/plain; charset="iso-8859-2"

I suppose the prerequisite for calling set_link() is that new node's prefix
should be longer than its parent's one (but still valid) - it's a rather
obvious way of building a prefix tree. So, prefix_bit() will always read
valid data.
________________________________
Od: quagga-dev-bounces@lists.quagga.net
<quagga-dev-bounces@lists.quagga.net> w imieniu u?ytkownika Matthias
Ferdinand <mf@14v.de>
Wys?ane: poniedzia?ek, 13 lipca 2020 14:29
Do: quagga-dev@lists.quagga.net <quagga-dev@lists.quagga.net>
Temat: [quagga-dev 16763] Re: possible problem with prefix_bit

On Mon, Jul 13, 2020 at 12:00:02PM +0100,
quagga-dev-request@lists.quagga.net wrote:
> Message: 1
> Date: Sun, 12 Jul 2020 21:22:15 +0000 (UTC)
> From: Kostas Sotiropoulos <kosotiro@yahoo.gr>
> To: "quagga-dev@lists.quagga.net" <quagga-dev@lists.quagga.net>
> Subject: [quagga-dev 16762] possible problem with prefix_bit
> Message-ID: <1148511041.807225.1594588935714@mail.yahoo.com>
> Content-Type: text/plain; charset="utf-8"
>
> Hi all,
>
> I do not know if this list is still valid but anyway I will express my
anxiety for a code snippet:
> Inside lib/table.c there is function set_link:
> static void
> set_link (struct route_node *node, struct route_node *new)
> {
> ? unsigned int bit = prefix_bit (&new->p.u.prefix, node->p.prefixlen);
>
> ? node->link[bit] = new;
> ? new->parent = node;
> }
> that calls function prefix_bit:
> unsigned int
> prefix_bit (const u_char *prefix, const u_char prefixlen)
> {
> ? unsigned int offset = prefixlen / 8;
> ? unsigned int shift? = 7 - (prefixlen % 8);
> ?
> ? return (prefix[offset] >> shift) & 1;
> }
>
> I suppose that prefixlen could also be equal to 32 for an IPV4 address
that could result to a buffer overrun insideprefix_bit. Am I right?
> Best regards,Kostas Sotiropoulos


[ Disclaimer: I'm not a developer ]

Hi,

note that struct prefix is at least 8 bytes long, and AFAICT is usually
embedded within a larger struct (e.g. struct route_node) with more
components following after struct prefix.

With 8 bytes size, prefixlen==32 for an IPv4 address will not read from
outside struct prefix (offset==4).

With prefixlen==128 for an IPv6 address it might read 1 byte after struct
prefix (offset==8), but still from valid memory.

Matthias Ferdinand
_______________________________________________
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.quagga.net/pipermail/quagga-dev/attachments/20200713/63da33f8/
attachment-0001.html>

------------------------------

_______________________________________________
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev

End of Quagga-dev Digest, Vol 188, Issue 2
******************************************


_______________________________________________
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev