NetBSD Problem Report #45661

From www@NetBSD.org  Sun Nov 27 08:25:13 2011
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by www.NetBSD.org (Postfix) with ESMTP id 13AAF63D970
	for <gnats-bugs@gnats.NetBSD.org>; Sun, 27 Nov 2011 08:25:13 +0000 (UTC)
Message-Id: <20111127082512.805A463D6CC@www.NetBSD.org>
Date: Sun, 27 Nov 2011 08:25:12 +0000 (UTC)
From: henning.petersen@t-online.de
Reply-To: henning.petersen@t-online.de
To: gnats-bugs@NetBSD.org
Subject: Overlapping buffer in route.c.
X-Send-Pr-Version: www-1.0

>Number:         45661
>Category:       bin
>Synopsis:       Overlapping buffer in route.c.
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    bin-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Nov 27 08:30:01 +0000 2011
>Closed-Date:    Tue Dec 27 20:26:19 +0000 2011
>Last-Modified:  Tue Dec 27 20:26:19 +0000 2011
>Originator:     Henning Petersen
>Release:        NetBSD-current
>Organization:
>Environment:
>Description:
Overlapping buffer in route.c with undefined behavior.
>How-To-Repeat:

>Fix:
diff -u -p -r1.134 route.c
--- sbin/route/route.c	11 Nov 2011 15:09:32 -0000	1.134
+++ sbin/route/route.c	19 Nov 2011 12:55:04 -0000
@@ -476,6 +476,7 @@ routename(const struct sockaddr *sa, str
 	static int first = 1;
 	struct in_addr in;
 	int nml;
+	size_t len;

 	if ((flags & RTF_HOST) == 0)
 		return netname(sa, nm);
@@ -599,10 +600,11 @@ routename(const struct sockaddr *sa, str

 		snprintf(line, sizeof(line), "%u", ms.shim.label);
 		pms = &((const struct sockaddr_mpls*)sa)->smpls_addr;
+		len = strlen(line);
 		while(psize < sa->sa_len) {
 			pms++;
 			ms.s_addr = ntohl(pms->s_addr);
-			snprintf(line, sizeof(line), "%s %u", line,
+			snprintf(line + len, sizeof(line) - len, " %u",
 			    ms.shim.label);
 			psize += sizeof(ms);
 		}

>Release-Note:

>Audit-Trail:
From: Christian Biere <christianbiere@gmx.de>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: bin/45661: Overlapping buffer in route.c
Date: Sun, 27 Nov 2011 14:02:13 +0100

 Unless I'm missing something the provided patch is incorrect
 because "len" is never updated inside the loop. However, I
 suggest using strlcat() instead to avoid pointer arithmetics
 with the destination buffer.

 There's also a similar issue in routename() which uses
 strlcpy() with overlapping memory.

 While we're at it, I also suggest fixing the alignment issues
 in three places.

 --- route.c.orig	2011-11-27 13:09:13.328002503 +0100
 +++ route.c	2011-11-27 13:57:01.928002817 +0100
 @@ -49,6 +49,7 @@ __RCSID("$NetBSD: route.c,v 1.134 2011/1
  #include <sys/ioctl.h>
  #include <sys/mbuf.h>
  #include <sys/sysctl.h>
 +#include <sys/endian.h>

  #include <net/if.h>
  #include <net/route.h>
 @@ -484,7 +485,7 @@ routename(const struct sockaddr *sa, str
  		first = 0;
  		if (gethostname(domain, MAXHOSTNAMELEN) == 0 &&
  		    (cp = strchr(domain, '.')))
 -			(void)strlcpy(domain, cp + 1, sizeof(domain));
 +			memmove(domain, &cp[1], strlen(cp) + 1);
  		else
  			domain[0] = 0;
  	}
 @@ -550,7 +551,7 @@ routename(const struct sockaddr *sa, str
  		     IN6_IS_ADDR_MC_LINKLOCAL(&sin6.sin6_addr)) &&
  		    sin6.sin6_scope_id == 0) {
  			sin6.sin6_scope_id =
 -			    ntohs(*(u_int16_t *)&sin6.sin6_addr.s6_addr[2]);
 +			    be16dec(&sin6.sin6_addr.s6_addr[2]);
  			sin6.sin6_addr.s6_addr[2] = 0;
  			sin6.sin6_addr.s6_addr[3] = 0;
  		}
 @@ -593,17 +594,19 @@ routename(const struct sockaddr *sa, str
  		union mpls_shim ms;
  		const union mpls_shim *pms;
  		int psize = sizeof(struct sockaddr_mpls);
 +		char label[32];

  		ms.s_addr =((const struct sockaddr_mpls*)sa)->smpls_addr.s_addr;
  		ms.s_addr = ntohl(ms.s_addr);

 -		snprintf(line, sizeof(line), "%u", ms.shim.label);
 +		line[0] = '\0';
 +		snprintf(label, sizeof label, "%u", ms.shim.label);
  		pms = &((const struct sockaddr_mpls*)sa)->smpls_addr;
  		while(psize < sa->sa_len) {
  			pms++;
  			ms.s_addr = ntohl(pms->s_addr);
 -			snprintf(line, sizeof(line), "%s %u", line,
 -			    ms.shim.label);
 +			snprintf(label, sizeof label, " %u", ms.shim.label);
 +			strlcat(line, sizeof line, label);
  			psize += sizeof(ms);
  		}
  		break;
 @@ -716,7 +719,7 @@ netname(const struct sockaddr *sa, struc
  		     IN6_IS_ADDR_MC_LINKLOCAL(&sin6.sin6_addr)) &&
  		    sin6.sin6_scope_id == 0) {
  			sin6.sin6_scope_id =
 -			    ntohs(*(u_int16_t *)&sin6.sin6_addr.s6_addr[2]);
 +				be16dec(&sin6.sin6_addr.s6_addr[2]);
  			sin6.sin6_addr.s6_addr[2] = 0;
  			sin6.sin6_addr.s6_addr[3] = 0;
  		}
 @@ -1270,8 +1273,8 @@ getaddr(int which, const char *s, struct
  		if ((IN6_IS_ADDR_LINKLOCAL(&su->sin6.sin6_addr) ||
  		     IN6_IS_ADDR_MC_LINKLOCAL(&su->sin6.sin6_addr)) &&
  		    su->sin6.sin6_scope_id) {
 -			*(u_int16_t *)&su->sin6.sin6_addr.s6_addr[2] =
 -				htons(su->sin6.sin6_scope_id);
 +			be16enc(&su->sin6.sin6_addr.s6_addr[2],
 +				su->sin6.sin6_scope_id);
  			su->sin6.sin6_scope_id = 0;
  		}
  #endif

From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/45661 CVS commit: src/sbin/route
Date: Sat, 24 Dec 2011 18:48:17 -0500

 Module Name:	src
 Committed By:	christos
 Date:		Sat Dec 24 23:48:17 UTC 2011

 Modified Files:
 	src/sbin/route: route.c

 Log Message:
 PR/45661: Henning Petersen: Overlapping buffer in route.c.


 To generate a diff of this commit:
 cvs rdiff -u -r1.134 -r1.135 src/sbin/route/route.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->feedback
State-Changed-By: wiz@NetBSD.org
State-Changed-When: Sun, 25 Dec 2011 13:51:06 +0000
State-Changed-Why:
Fixed?


From: henning petersen <henning.petersen@t-online.de>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: bin/45661 (Overlapping buffer in route.c.)
Date: Sun, 25 Dec 2011 15:07:52 +0100

 Am 25.12.2011 14:51, schrieb wiz@NetBSD.org:
 > Synopsis: Overlapping buffer in route.c.
 >
 > State-Changed-From-To: open->feedback
 > State-Changed-By: wiz@NetBSD.org
 > State-Changed-When: Sun, 25 Dec 2011 13:51:06 +0000
 > State-Changed-Why:
 > Fixed?
 >
 >
 >
 >
 Fixed OK,

State-Changed-From-To: feedback->closed
State-Changed-By: wiz@NetBSD.org
State-Changed-When: Tue, 27 Dec 2011 20:26:19 +0000
State-Changed-Why:
Fixed, thank you!


>Unformatted:

Home	PR Database Search