nd6_rtr.c possible buffer overflow ?

[Loganaden Velvindron]

Hi All,


From nd6_rtr.c:

memset(&ifra, 0, sizeof(ifra));
/*
* in6_update_ifa() does not use ifra_name, but we accurately set it
* for safety.
*/
strncpy(ifra.ifra_name, if_name(ifp), sizeof(ifra.ifra_name));
sockaddr_in6_init(&ifra.ifra_addr, &pr->ndpr_prefix.sin6_addr, 0, 0, 0);
/* prefix */
ifra.ifra_addr.sin6_addr.s6_addr32[0] &= mask.s6_addr32[0];
ifra.ifra_addr.sin6_addr.s6_addr32[1] &= mask.s6_addr32[1];
ifra.ifra_addr.sin6_addr.s6_addr32[2] &= mask.s6_addr32[2];
ifra.ifra_addr.sin6_addr.s6_addr32[3] &= mask.s6_addr32[3];

Assuming that if_name(ifp) is the maximum size, wouldn't that possibly lead to
an unterminated string.

In such a case, wouldn't strlcpy be better ?

Index: src/sys/netinet6/nd6_rtr.c
===================================================================
RCS file: /cvsroot/src/sys/netinet6/nd6_rtr.c,v
retrieving revision 1.90
diff -u -p -r1.90 nd6_rtr.c
--- src/sys/netinet6/nd6_rtr.c 14 Sep 2013 21:08:35 -0000 1.90
+++ src/sys/netinet6/nd6_rtr.c 3 Oct 2013 15:50:40 -0000
@@ -1868,7 +1868,7 @@ in6_ifadd(struct nd_prefixctl *pr, int m
* in6_update_ifa() does not use ifra_name, but we accurately set it
* for safety.
*/
- strncpy(ifra.ifra_name, if_name(ifp), sizeof(ifra.ifra_name));
+ strlcpy(ifra.ifra_name, if_name(ifp), sizeof(ifra.ifra_name));
sockaddr_in6_init(&ifra.ifra_addr, &pr->ndpr_prefix.sin6_addr, 0, 0, 0);
/* prefix */
ifra.ifra_addr.sin6_addr.s6_addr32[0] &= mask.s6_addr32[0];

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[Joerg Sonnenberger]

Post by Loganaden Velvindron
Assuming that if_name(ifp) is the maximum size, wouldn't that possibly lead to
an unterminated string.

Maximum size includes the \0?

Post by Loganaden Velvindron
In such a case, wouldn't strlcpy be better ?

Not for the given reason, but it would be slightly less awful in terms
of performance as ifra is already zero initialised before. If changing
it, it should still get an explicit comment about that as we don't want
to leak content of the kernel stack.

Joerg

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de