NetBSD Problem Report #45662

From www@NetBSD.org  Sun Nov 27 08:27:49 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 7890963D970
	for <gnats-bugs@gnats.NetBSD.org>; Sun, 27 Nov 2011 08:27:49 +0000 (UTC)
Message-Id: <20111127082748.DF74E63D6CC@www.NetBSD.org>
Date: Sun, 27 Nov 2011 08:27:48 +0000 (UTC)
From: henning.petersen@t-online.de
Reply-To: henning.petersen@t-online.de
To: gnats-bugs@NetBSD.org
Subject: Overlapping buffer in lpd_command.c.
X-Send-Pr-Version: www-1.0

>Number:         45662
>Category:       bin
>Synopsis:       Overlapping buffer in ldp_command.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:    Sun Dec 25 01:08:27 +0000 2011
>Last-Modified:  Tue Jan 29 02:55:08 +0000 2013
>Originator:     Henning Petersen
>Release:        NetBSD-current
>Organization:
>Environment:
>Description:
Overlapping buffer in ldp_command.c with undefined behavior if snprintf is
used.
>How-To-Repeat:

>Fix:
diff -u -p -r1.5 ldp_command.c
--- usr.sbin/ldpd/ldp_command.c	16 Jun 2011 08:27:28 -0000	1.5
+++ usr.sbin/ldpd/ldp_command.c	20 Nov 2011 05:54:33 -0000
@@ -481,6 +481,7 @@ show_labels(int s, char *recvspace)
 {
 	struct ldp_peer *p;
 	struct label_mapping *lm;
+	size_t len;

 	SLIST_FOREACH(p, &ldp_peer_head, peers) {
 		if (p->state != LDP_PEER_ESTABLISHED)
@@ -488,8 +489,10 @@ show_labels(int s, char *recvspace)
 		SLIST_FOREACH(lm, &p->label_mapping_head, mappings) {
 			snprintf(sendspace, MAXSEND, "%s:%d",
 			    inet_ntoa(p->ldp_id), lm->label);
-			snprintf(sendspace, MAXSEND, "%s\t%s/%d\n",
-			    sendspace, inet_ntoa(lm->address), lm->prefix);
+			len = strlen(sendspace);
+			snprintf(sendspace + len, MAXSEND - len,
+				"\t%s/%d\n",
+			    inet_ntoa(lm->address), lm->prefix);
 			writestr(s, sendspace);
 		}
 	}

>Release-Note:

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

 I suggest just merging the two snprintf() calls instead to avoid
 any pointer arithmetic with the destination buffer.

 --- ldp_command.c.orig	2011-11-27 12:53:00.338002396 +0100
 +++ ldp_command.c	2011-11-27 12:57:08.228002423 +0100
 @@ -486,10 +486,9 @@ show_labels(int s, char *recvspace)
  		if (p->state != LDP_PEER_ESTABLISHED)
  			continue;
  		SLIST_FOREACH(lm, &p->label_mapping_head, mappings) {
 -			snprintf(sendspace, MAXSEND, "%s:%d",
 -			    inet_ntoa(p->ldp_id), lm->label);
 -			snprintf(sendspace, MAXSEND, "%s\t%s/%d\n",
 -			    sendspace, inet_ntoa(lm->address), lm->prefix);
 +			snprintf(sendspace, MAXSEND, "%s:%d\t%s/%d\n",
 +			    inet_ntoa(p->ldp_id), lm->label,
 +			    inet_ntoa(lm->address), lm->prefix);
  			writestr(s, sendspace);
  		}
  	}

From: henning petersen <henning.petersen@t-online.de>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: bin/45662: Overlapping buffer in lpd_command.c.
Date: Mon, 05 Dec 2011 11:12:02 +0100

 This is a multi-part message in MIME format.
 --------------080607040907030007050007
 Content-Type: text/plain; charset=ISO-8859-15
 Content-Transfer-Encoding: 7bit

 Am 27.11.2011 13:10, schrieb Christian Biere:
 > The following reply was made to PR bin/45662; it has been noted by GNATS.
 >
 > From: Christian Biere <christianbiere@gmx.de>
 > To: gnats-bugs@NetBSD.org
 > Cc: 
 > Subject: Re: bin/45662: Overlapping buffer in lpd_command.c.
 > Date: Sun, 27 Nov 2011 13:06:31 +0100
 >
 >  I suggest just merging the two snprintf() calls instead to avoid
 >  any pointer arithmetic with the destination buffer.
 >  
 >  --- ldp_command.c.orig	2011-11-27 12:53:00.338002396 +0100
 >  +++ ldp_command.c	2011-11-27 12:57:08.228002423 +0100
 >  @@ -486,10 +486,9 @@ show_labels(int s, char *recvspace)
 >   		if (p->state != LDP_PEER_ESTABLISHED)
 >   			continue;
 >   		SLIST_FOREACH(lm, &p->label_mapping_head, mappings) {
 >  -			snprintf(sendspace, MAXSEND, "%s:%d",
 >  -			    inet_ntoa(p->ldp_id), lm->label);
 >  -			snprintf(sendspace, MAXSEND, "%s\t%s/%d\n",
 >  -			    sendspace, inet_ntoa(lm->address), lm->prefix);
 >  +			snprintf(sendspace, MAXSEND, "%s:%d\t%s/%d\n",
 >  +			    inet_ntoa(p->ldp_id), lm->label,
 >  +			    inet_ntoa(lm->address), lm->prefix);
 >   			writestr(s, sendspace);
 >   		}
 >   	}
 >  
 >
 OK Attach is the new patch.

 --------------080607040907030007050007
 Content-Type: text/plain;
  name="ldp_command.c.diff"
 Content-Transfer-Encoding: 7bit
 Content-Disposition: attachment;
  filename="ldp_command.c.diff"

 diff -u -p -r1.5 ldp_command.c
 --- usr.sbin/ldpd/ldp_command.c	16 Jun 2011 08:27:28 -0000	1.5
 +++ usr.sbin/ldpd/ldp_command.c	27 Nov 2011 17:52:42 -0000
 @@ -486,10 +486,10 @@ show_labels(int s, char *recvspace)
  		if (p->state != LDP_PEER_ESTABLISHED)
  			continue;
  		SLIST_FOREACH(lm, &p->label_mapping_head, mappings) {
 -			snprintf(sendspace, MAXSEND, "%s:%d",
 -			    inet_ntoa(p->ldp_id), lm->label);
 -			snprintf(sendspace, MAXSEND, "%s\t%s/%d\n",
 -			    sendspace, inet_ntoa(lm->address), lm->prefix);
 +			snprintf(sendspace, MAXSEND,
 +				"%s:%d\t%s/%d\n",
 +			    inet_ntoa(p->ldp_id), lm->label,
 +			    inet_ntoa(lm->address), lm->prefix);
  			writestr(s, sendspace);
  		}
  	}

 --------------080607040907030007050007--

From: David Laight <david@l8s.co.uk>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: bin/45662: Overlapping buffer in lpd_command.c.
Date: Mon, 5 Dec 2011 18:12:50 +0000

 On Sun, Nov 27, 2011 at 12:10:07PM +0000, Christian Biere wrote:
 > The following reply was made to PR bin/45662; it has been noted by GNATS.
 > 
 > From: Christian Biere <christianbiere@gmx.de>
 > To: gnats-bugs@NetBSD.org
 > Cc: 
 > Subject: Re: bin/45662: Overlapping buffer in lpd_command.c.
 > Date: Sun, 27 Nov 2011 13:06:31 +0100
 > 
 >  I suggest just merging the two snprintf() calls instead to avoid
 >  any pointer arithmetic with the destination buffer.
 >  
 >  --- ldp_command.c.orig	2011-11-27 12:53:00.338002396 +0100
 >  +++ ldp_command.c	2011-11-27 12:57:08.228002423 +0100
 >  @@ -486,10 +486,9 @@ show_labels(int s, char *recvspace)
 >   		if (p->state != LDP_PEER_ESTABLISHED)
 >   			continue;
 >   		SLIST_FOREACH(lm, &p->label_mapping_head, mappings) {
 >  -			snprintf(sendspace, MAXSEND, "%s:%d",
 >  -			    inet_ntoa(p->ldp_id), lm->label);
 >  -			snprintf(sendspace, MAXSEND, "%s\t%s/%d\n",
 >  -			    sendspace, inet_ntoa(lm->address), lm->prefix);
 >  +			snprintf(sendspace, MAXSEND, "%s:%d\t%s/%d\n",
 >  +			    inet_ntoa(p->ldp_id), lm->label,
 >  +			    inet_ntoa(lm->address), lm->prefix);

 inet_ntoa() will be using the same output buffer - so this
 would print the same string twice.

 	David

 -- 
 David Laight: david@l8s.co.uk

From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/45662 CVS commit: src/usr.sbin/ldpd
Date: Sat, 24 Dec 2011 18:51:27 -0500

 Module Name:	src
 Committed By:	christos
 Date:		Sat Dec 24 23:51:27 UTC 2011

 Modified Files:
 	src/usr.sbin/ldpd: ldp_command.c

 Log Message:
 PR/45662: Henning Petersen: Overlapping buffer in lpd_command.c


 To generate a diff of this commit:
 cvs rdiff -u -r1.5 -r1.6 src/usr.sbin/ldpd/ldp_command.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->closed
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Sun, 25 Dec 2011 01:08:27 +0000
State-Changed-Why:
fixed, thakns
(yes, the final form addresses dsl@'s observation; see source-changes)


>Unformatted:

Home	PR Database Search