NetBSD Problem Report #45660

From www@NetBSD.org  Sun Nov 27 08:22:24 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 3585E63D970
	for <gnats-bugs@gnats.NetBSD.org>; Sun, 27 Nov 2011 08:22:24 +0000 (UTC)
Message-Id: <20111127082223.8F04263D6CC@www.NetBSD.org>
Date: Sun, 27 Nov 2011 08:22:23 +0000 (UTC)
From: henning.petersen@t-online.de
Reply-To: henning.petersen@t-online.de
To: gnats-bugs@NetBSD.org
Subject: Overlapping buffer in catman.c.
X-Send-Pr-Version: www-1.0

>Number:         45660
>Category:       bin
>Synopsis:       Overlapping buffer in catman.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:25:00 +0000 2011
>Closed-Date:    Sun Dec 25 00:47:29 +0000 2011
>Last-Modified:  Tue Dec 27 12:50:02 +0000 2011
>Originator:     Henning Petersen
>Release:        NetBSD-current
>Organization:
>Environment:
>Description:
Overlapping buffer in catman.c with undefined behavior if s[n]printf is
used.
>How-To-Repeat:

>Fix:
diff -u -p -r1.30 catman.c
--- usr.sbin/catman/catman.c	29 Aug 2011 20:38:55 -0000	1.30
+++ usr.sbin/catman/catman.c	19 Nov 2011 12:50:21 -0000
@@ -561,14 +561,16 @@ makecat(const char *manpage, const char 
 {
 	char crunchbuf[1024];
 	char sysbuf[2048];
+	size_t 	len;

 	snprintf(sysbuf, sizeof(sysbuf), buildcmd, manpage);

+	len = strlen(sysbuf);
 	if (*crunchcmd != '\0') {
 		snprintf(crunchbuf, sizeof(crunchbuf), crunchcmd, catpage);
-		snprintf(sysbuf, sizeof(sysbuf), "%s | %s", sysbuf, crunchbuf);
+		snprintf(sysbuf + len, sizeof(sysbuf) - len, " | %s", crunchbuf);
 	} else {
-		snprintf(sysbuf, sizeof(sysbuf), "%s > %s", sysbuf, catpage);
+		snprintf(sysbuf + len, sizeof(sysbuf) - len, " > %s", catpage);
 	}

 	if (f_noprint == 0)

>Release-Note:

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

 I assume the missing constant format string argument at the first
 snprintf() in makecat() is actually a bug. I'm sure the compiler
 is warning about this.

 Therefore, I propose the following patch instead.

 --- catman.c.orig	2011-11-27 14:27:47.788003030 +0100
 +++ catman.c	2011-11-27 14:27:32.728003016 +0100
 @@ -559,16 +559,17 @@ static void
  makecat(const char *manpage, const char *catpage, const char *buildcmd, 
  	const char *crunchcmd)
  {
 -	char crunchbuf[1024];
  	char sysbuf[2048];

 -	snprintf(sysbuf, sizeof(sysbuf), buildcmd, manpage);
 -
  	if (*crunchcmd != '\0') {
 +		char crunchbuf[1024];
 +
  		snprintf(crunchbuf, sizeof(crunchbuf), crunchcmd, catpage);
 -		snprintf(sysbuf, sizeof(sysbuf), "%s | %s", sysbuf, crunchbuf);
 +		snprintf(sysbuf, sizeof(sysbuf), "%s%s | %s",
 +			buildcmd, manpage, crunchbuf);
  	} else {
 -		snprintf(sysbuf, sizeof(sysbuf), "%s > %s", sysbuf, catpage);
 +		snprintf(sysbuf, sizeof(sysbuf), "%s%s > %s",
 +			buildcmd, manpage, catpage);
  	}

  	if (f_noprint == 0)


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

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

 Am 27.11.2011 14:35, schrieb Christian Biere:
 > The following reply was made to PR bin/45660; it has been noted by GNATS.
 >
 > From: Christian Biere <christianbiere@gmx.de>
 > To: gnats-bugs@NetBSD.org
 > Cc: 
 > Subject: Re: bin/45660: Overlapping buffer in catman.c.
 > Date: Sun, 27 Nov 2011 14:33:18 +0100
 >
 >  I assume the missing constant format string argument at the first
 >  snprintf() in makecat() is actually a bug. I'm sure the compiler
 >  is warning about this.
 >  
 >  Therefore, I propose the following patch instead.
 >   
 >  --- catman.c.orig	2011-11-27 14:27:47.788003030 +0100
 >  +++ catman.c	2011-11-27 14:27:32.728003016 +0100
 >  @@ -559,16 +559,17 @@ static void
 >   makecat(const char *manpage, const char *catpage, const char *buildcmd, 
 >   	const char *crunchcmd)
 >   {
 >  -	char crunchbuf[1024];
 >   	char sysbuf[2048];
 >   
 >  -	snprintf(sysbuf, sizeof(sysbuf), buildcmd, manpage);
 >  -
 >   	if (*crunchcmd != '\0') {
 >  +		char crunchbuf[1024];
 >  +
 >   		snprintf(crunchbuf, sizeof(crunchbuf), crunchcmd, catpage);
 >  -		snprintf(sysbuf, sizeof(sysbuf), "%s | %s", sysbuf, crunchbuf);
 >  +		snprintf(sysbuf, sizeof(sysbuf), "%s%s | %s",
 >  +			buildcmd, manpage, crunchbuf);
 >   	} else {
 >  -		snprintf(sysbuf, sizeof(sysbuf), "%s > %s", sysbuf, catpage);
 >  +		snprintf(sysbuf, sizeof(sysbuf), "%s%s > %s",
 >  +			buildcmd, manpage, catpage);
 >   	}
 >   
 >   	if (f_noprint == 0)
 >  
 >  
 >
 OK Attach is the new patch.

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

 diff -u -p -r1.30 catman.c
 --- usr.sbin/catman/catman.c	29 Aug 2011 20:38:55 -0000	1.30
 +++ usr.sbin/catman/catman.c	27 Nov 2011 17:51:36 -0000
 @@ -559,16 +559,16 @@ static void
  makecat(const char *manpage, const char *catpage, const char *buildcmd, 
  	const char *crunchcmd)
  {
 -	char crunchbuf[1024];
  	char sysbuf[2048];

 -	snprintf(sysbuf, sizeof(sysbuf), buildcmd, manpage);
 -
  	if (*crunchcmd != '\0') {
 +		char crunchbuf[1024];
  		snprintf(crunchbuf, sizeof(crunchbuf), crunchcmd, catpage);
 -		snprintf(sysbuf, sizeof(sysbuf), "%s | %s", sysbuf, crunchbuf);
 +		snprintf(sysbuf, sizeof(sysbuf), "%s%s | %s",
 +				buildcmd, manpage, crunchbuf);
  	} else {
 -		snprintf(sysbuf, sizeof(sysbuf), "%s > %s", sysbuf, catpage);
 +		snprintf(sysbuf, sizeof(sysbuf), "%s%s > %s",
 +				buildcmd, manpage, catpage);
  	}

  	if (f_noprint == 0)

 --------------030105010608030801060308--

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

 Module Name:	src
 Committed By:	christos
 Date:		Sat Dec 24 23:29:32 UTC 2011

 Modified Files:
 	src/usr.sbin/catman: catman.c

 Log Message:
 PR/45660: Henning Petersen: Overlapping buffer in catman.c.


 To generate a diff of this commit:
 cvs rdiff -u -r1.30 -r1.31 src/usr.sbin/catman/catman.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 00:47:29 +0000
State-Changed-Why:
fixed, thanks


From: Alan Barrett <apb@cequrux.com>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: bin/45660: Overlapping buffer in catman.c.
Date: Tue, 27 Dec 2011 14:46:13 +0200

 On Sun, 27 Nov 2011, Christian Biere wrote:
 > I assume the missing constant format string argument at the first
 > snprintf() in makecat() is actually a bug. I'm sure the compiler
 > is warning about this.

 The buildcmd argument to the makecat() function in catman.c is 
 taken from a "_build" line in man.conf, in which "%s" stands for 
 the name of the file to be formatted.  Using this string as a 
 format argument to sprintf is unsafe (there may be escapes other 
 than %s, or more than one %s), but it's less wrong than treating 
 it as a constant string and not expanding any %s at all.

 --apb (Alan Barrett)

>Unformatted:

Home	PR Database Search