Mailing List Archive

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug
report.

http://bugzilla.quagga.net/show_bug.cgi?id=452

Summary: zebra: stack overflow in function rtm_read when reading
initial route
Product: Quagga
Version: 0.99.9
Platform: PC
OS/Version: OpenBSD
Status: UNCONFIRMED
Severity: blocker
Priority: High
Component: zebra
AssignedTo: maintainers@quagga.net
ReportedBy: raph@futomaki.net


Hello,

I recently upgrade several box to openbsd 4.3 and I upgrade the quagga port too.
The version in the port tree is the 0.99.9. Every I launch the zebra daemon, It
directly crash makih an "abort trap", and zebra: stack overflow in function
rtm_read in syslog. I have the same behavior on every box I tried.

I locate the rtm_read function, and I spend some hours to debug what are wrong.
Finaly this I was I found, some message passed in the RTA_ATTR_GET macro(reading
netmask infos) are too long, and the "memcpy (pdest, (PNT), len);" make an
overflow which are trapped by Openbsd.

This is the modified macro to the bug in evidence :

#define RTA_ATTR_GET(DEST, RTA, RTMADDRS, PNT) \
if ((RTMADDRS) & (RTA)) \
{ \
void *pdest = (DEST); \
int len = SAROUNDUP ((PNT)); \
if ((DEST) != NULL) {\
printf ("{%d} ",((struct sockaddr *)(PNT))->sa_len); \
printf ("[%d] ",len); \
if (len <= sizeof (union sockunion) ) \
memcpy (pdest, (PNT), len); \
else \
printf("bug"); \
}\
(PNT) += len; \
}

I also add a printf directely in the rtm_read_mesg.
This the ouptut on my test box :

<snip>
=> lo0 255.255.255.255 {0} [4]
=> lo0 0.0.0.0 {0} [4]
=> lo0 0.0.0.0 {0} [4]
=> gif0 0.0.0.0
=> gif0 0.0.0.0
=> lo0 0.0.0.0 {21} [24]
=> lo0 255.255.255.255 {127} [128] bug
=> lo0 0.0.0.0 {21} [24]
=> lo0 255.255.255.255 {224} [224] bug
=> lo0 0.0.0.0 {21} [24]
=> lo0 255.255.255.255 {255} [256] bug
=> lo0 0.0.0.0 {20} [20]
=> lo0 255.255.255.255 {0} [4]
=> lo0 0.0.0.0
=> lo0 0.0.0.0
<snip>

Notice the lines with the bug word.
The lenght of (struct sockaddr *)(PNT))->sa_len is too much for the union
sockunion which it 28 long at maximum (with ipv6).
I don't know if these type of kernel route information are normal or not ?
but to fix this I propose the following modification :

#define RTA_ATTR_GET(DEST, RTA, RTMADDRS, PNT) \
if ((RTMADDRS) & (RTA)) \
{ \
void *pdest = (DEST); \
int len = SAROUNDUP ((PNT)); \
if (((DEST) != NULL) && (len <= sizeof (union sockunion)) \
memcpy (pdest, (PNT), len); \
(PNT) += len; \
}

It seems a good idea to test the size of the data in memcpy.
Any comment ?



------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
_______________________________________________
Quagga-bugs mailing list
Quagga-bugs@lists.quagga.net
http://lists.quagga.net/mailman/listinfo/quagga-bugs