NetBSD Problem Report #45633

From www@NetBSD.org  Sat Nov 19 12:32:56 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 3403F63CD4E
	for <gnats-bugs@gnats.NetBSD.org>; Sat, 19 Nov 2011 12:32:56 +0000 (UTC)
Message-Id: <20111119123255.5A45E63CD2A@www.NetBSD.org>
Date: Sat, 19 Nov 2011 12:32:55 +0000 (UTC)
From: christianbiere@gmx.de
Reply-To: christianbiere@gmx.de
To: gnats-bugs@NetBSD.org
Subject: Improper string handling in cnmagic.c
X-Send-Pr-Version: www-1.0

>Number:         45633
>Category:       kern
>Synopsis:       Improper string handling in cnmagic.c
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    kern-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sat Nov 19 12:35:00 +0000 2011
>Closed-Date:    Sun Jan 22 15:37:14 +0000 2012
>Last-Modified:  Sun Jan 22 15:37:14 +0000 2012
>Originator:     Christian Biere
>Release:        
>Organization:
>Environment:
>Description:
File: sys/kern/cnmagic.c
Function: cn_set_magic()

1. The code accesses the byte after the NUL byte of "magic".
2. The code assigns cn_magic[i] once from uninitalized memory m[i].

Function: cn_get_magic()

3. The length restriction by the parameter maglen is completely ignored.
4. If cn_magic_set() was called with an empty string "" as parameter, it is expanded to "\x27\x02".


>How-To-Repeat:

>Fix:

>Release-Note:

>Audit-Trail:
From: Christian Biere <christianbiere@gmx.de>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: kern/45633: Improper string handling in cnmagic.c
Date: Sat, 19 Nov 2011 13:39:38 +0100

 This is a multi-part message in MIME format.

 --Multipart=_Sat__19_Nov_2011_13_39_38_+0100_kbNCZFi0ZSMa+YW2
 Content-Type: text/plain; charset=US-ASCII
 Content-Transfer-Encoding: 7bit

 I propose the following changes.
 See attached unified diff. 

 -- 
 Christian Biere

 --Multipart=_Sat__19_Nov_2011_13_39_38_+0100_kbNCZFi0ZSMa+YW2
 Content-Type: text/plain;
  name="cnmagic.c.udif"
 Content-Disposition: attachment;
  filename="cnmagic.c.udif"
 Content-Transfer-Encoding: quoted-printable

 --- cnmagic.c.orig	2011-11-19 11:44:13.312079005 +0100
 +++ cnmagic.c	2011-11-19 13:17:31.542080915 +0100
 @@ -68,26 +68,23 @@
 =20
  	for (i =3D 0; i < CNS_LEN; i++) {
  		c =3D (*magic++) & 0xff;
 -		n =3D *magic ? i+1 : CNS_TERM;
  		switch (c) {
  		case 0:
  			/* End of string */
  			if (i =3D=3D 0) {
  				/* empty string? */
 -				cn_magic[0] =3D 0;
  #ifdef DEBUG
  				printf("cn_set_magic(): empty!\n");
  #endif
 -				return (0);
  			}
 -			do {
 +			cn_magic[i] =3D 0;
 +			while (i--) {
  				cn_magic[i] =3D m[i];
 -			} while (i--);
 +			}
  			return(0);
  		case 0x27:
  			/* Escape sequence */
  			c =3D (*magic++) & 0xff;
 -			n =3D *magic ? i+1 : CNS_TERM;
  			switch (c) {
  			case 0x27:
  				break;
 @@ -103,6 +100,7 @@
  			/* FALLTHROUGH */
  		default:
  			/* Transition to the next state. */
 +			n =3D *magic ? i+1 : CNS_TERM;
  #ifdef DEBUG
  			if (!cold)
  				aprint_normal("mag %d %x:%x\n", i, c, n);
 @@ -121,36 +119,51 @@
  int
  cn_get_magic(char *magic, size_t maglen)
  {
 -	size_t i, c;
 +	size_t i, n =3D 0;
 +
 +#define ADD_CHAR(x) \
 +do { \
 +	if (n < maglen) \
 +		magic[n++] =3D (x); \
 +	else \
 +		goto error; \
 +} while (0)=20
 +
 +	for (i =3D 0; i < CNS_LEN; /* empty */) {
 +		unsigned short c =3D cn_magic[i];
 +		i =3D CNS_MAGIC_NEXT(c);
 +		if (i =3D=3D 0)
 +			goto finish;
 =20
 -	for (i =3D 0; i < CNS_LEN;) {
 -		c =3D cn_magic[i];
  		/* Translate a character */
  		switch (CNS_MAGIC_VAL(c)) {
  		case CNC_BREAK:
 -			*magic++ =3D 0x27;
 -			*magic++ =3D 0x01;
 +			ADD_CHAR(0x27);
 +			ADD_CHAR(0x01);
  			break;
  		case 0:
 -			*magic++ =3D 0x27;
 -			*magic++ =3D 0x02;
 +			ADD_CHAR(0x27);
 +			ADD_CHAR(0x02);
  			break;
  		case 0x27:
 -			*magic++ =3D 0x27;
 -			*magic++ =3D 0x27;
 +			ADD_CHAR(0x27);
 +			ADD_CHAR(0x27);
  			break;
  		default:
 -			*magic++ =3D (c & 0x0ff);
 +			ADD_CHAR(c & 0x0ff);
  			break;
  		}
  		/* Now go to the next state */
 -		i =3D CNS_MAGIC_NEXT(c);
 -		if (i =3D=3D CNS_TERM || i =3D=3D 0) {
 -			/* Either termination state or empty machine */
 -			*magic++ =3D 0;
 -			return (0);
 -		}
 +		if (i =3D=3D CNS_TERM)
 +			goto finish;
  	}
 +
 +error:
  	return (EINVAL);
 +
 +finish:
 +	/* Either termination state or empty machine */
 +	ADD_CHAR('\0');
 +	return (0);
  }
 =20

 --Multipart=_Sat__19_Nov_2011_13_39_38_+0100_kbNCZFi0ZSMa+YW2--

From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/45633 CVS commit: src/sys/kern
Date: Sat, 19 Nov 2011 11:11:25 -0500

 Module Name:	src
 Committed By:	christos
 Date:		Sat Nov 19 16:11:24 UTC 2011

 Modified Files:
 	src/sys/kern: cnmagic.c

 Log Message:
 PR/45633: Christian Biere: Don't access byte after NUL when setting magic.


 To generate a diff of this commit:
 cvs rdiff -u -r1.11 -r1.12 src/sys/kern/cnmagic.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, 22 Jan 2012 15:37:14 +0000
State-Changed-Why:
Christos committed it, thanks.


>Unformatted:

Home	PR Database Search