NetBSD Problem Report #44008

From itohy@netbsd.org  Fri Oct 29 04:22:59 2010
Return-Path: <itohy@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by www.NetBSD.org (Postfix) with ESMTP id 6D57A63B8F2
	for <gnats-bugs@gnats.NetBSD.org>; Fri, 29 Oct 2010 04:22:59 +0000 (UTC)
Message-Id: <201010290422.o9T4MtJY018159@v057181.ppp.asahi-net.or.jp>
Date: Fri, 29 Oct 2010 13:22:55 +0900 (JST)
From: itohy@netbsd.org
Reply-To: itohy@netbsd.org
To: gnats-bugs@gnats.NetBSD.org
Subject: long boot.cfg command overruns buffer
X-Send-Pr-Version: 3.95

>Number:         44008
>Category:       port-i386
>Synopsis:       long boot.cfg command overruns buffer
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    port-i386-maintainer
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Oct 29 04:25:00 +0000 2010
>Last-Modified:  Sun Oct 02 19:35:01 +0000 2016
>Originator:     ITOH Yasufumi
>Release:        NetBSD 5.99.39
>Organization:
>Environment:
System: NetBSD zun 5.99.39 NetBSD 5.99.39 (ZUN3PAE_DOM0) #2: Fri Oct 15 20:32:08 JST 2010 itohy@zun:/usr/src/current/sys/arch/i386/compile/ZUN3PAE_DOM0 i386
Architecture: i386
Machine: i386
amd64 is also affected.

>Description:
	The length of a boot.cfg command is limited to ~80 bytes,
	which is rather too short.
	If the length is longer than ~80bytes, it corrupts stack.

>How-To-Repeat:
	Try following line in /boot.cfg and boot XEN.

menu=Boot Xen with 3.5GB for dom0:load /netbsd-XEN3PAE_DOM0 console=pc pciback.hide=(00:1a.0)(00:1a.1)(00:1a.2)(00:1a.7); multiboot /xen.gz dom0_mem=3584M

	dmesg:

pci0: i/o space, memory space enabled
pciback_pci_init: hide claim device 0:1a:0
pciback_pci_init: hide claim device 0:1a:1
pciback_pci_init: hide claim device 0:1a:2
pciback_pci_init: syntax error at 00:1a.
...

	Note the load command exeeds 80byte long and pciback.hide string
	is cut in the midle.

>Fix:
	At least sys/arch/i386/stand/lib/bootmenu.c::doboottypemenu()
	copies a command into 80byte buffer (without boundary checking)
	and needs fixed.

	Here's untested patch.  The maximum size of boot.ini is 32KB.

Index: bootmenu.c
===================================================================
RCS file: /cvsroot/src/sys/arch/i386/stand/lib/bootmenu.c,v
retrieving revision 1.8
diff -u -p -r1.8 bootmenu.c
--- bootmenu.c	14 Sep 2009 10:42:42 -0000	1.8
+++ bootmenu.c	24 Oct 2010 04:19:31 -0000
@@ -290,7 +290,7 @@ void
 doboottypemenu(void)
 {
 	int choice;
-	char input[80], *ic, *oc;
+	char input[32768], *ic, *oc;

 	printf("\n");
 	/* Display menu */

>Audit-Trail:
From: David Holland <dholland-bugs@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: port-i386/44008: long boot.cfg command overruns buffer
Date: Sun, 2 Oct 2016 19:14:24 +0000

 On Fri, Oct 29, 2010 at 04:25:01AM +0000, itohy@netbsd.org wrote:
  > 	The length of a boot.cfg command is limited to ~80 bytes,
  > 	which is rather too short.
  > 	If the length is longer than ~80bytes, it corrupts stack.
  > 
  > >How-To-Repeat:
  > 	Try following line in /boot.cfg and boot XEN.
  > 
  > menu=Boot Xen with 3.5GB for dom0:load /netbsd-XEN3PAE_DOM0 console=pc pciback.hide=(00:1a.0)(00:1a.1)(00:1a.2)(00:1a.7); multiboot /xen.gz dom0_mem=3584M

 ok...

 but,

  > 	At least sys/arch/i386/stand/lib/bootmenu.c::doboottypemenu()
  > 	copies a command into 80byte buffer (without boundary checking)
  > 	and needs fixed.
  > 
  > 	Here's untested patch.  The maximum size of boot.ini is 32KB.
  > 
  > Index: bootmenu.c
  > ===================================================================
  > RCS file: /cvsroot/src/sys/arch/i386/stand/lib/bootmenu.c,v
  > retrieving revision 1.8
  > diff -u -p -r1.8 bootmenu.c
  > --- bootmenu.c	14 Sep 2009 10:42:42 -0000	1.8
  > +++ bootmenu.c	24 Oct 2010 04:19:31 -0000
  > @@ -290,7 +290,7 @@ void
  >  doboottypemenu(void)
  >  {
  >  	int choice;
  > -	char input[80], *ic, *oc;
  > +	char input[32768], *ic, *oc;
  >  
  >  	printf("\n");
  >  	/* Display menu */

 I don't see any way boot.cfg data can get into this buffer, which is
 used only for console input. (which nowadays is bounded properly)

 -- 
 David A. Holland
 dholland@netbsd.org

From: David Holland <dholland-bugs@netbsd.org>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: port-i386/44008: long boot.cfg command overruns buffer
Date: Sun, 2 Oct 2016 19:30:03 +0000

 On Sun, Oct 02, 2016 at 07:15:01PM +0000, David Holland wrote:
  >  I don't see any way boot.cfg data can get into this buffer, which is
  >  used only for console input. (which nowadays is bounded properly)

 oh, wait, I see. It's not that buffer any more.

 untested candidate patch:

 Index: bootmenu.c
 ===================================================================
 RCS file: /cvsroot/src/sys/arch/i386/stand/lib/bootmenu.c,v
 retrieving revision 1.16
 diff -u -p -r1.16 bootmenu.c
 --- bootmenu.c	11 Jun 2016 06:20:11 -0000	1.16
 +++ bootmenu.c	2 Oct 2016 19:28:38 -0000
 @@ -103,25 +103,32 @@ getchoicefrominput(char *input, int def)
  static void
  docommandchoice(int choice)
  {
 -	char input[80], *ic, *oc;
 +	char input[256], *ic;
 +	size_t pos;

  	ic = bootcfg_info.command[choice];
  	/* Split command string at ; into separate commands */
  	do {
 -		oc = input;
 +		pos = 0;
  		/* Look for ; separator */
 -		for (; *ic && *ic != COMMAND_SEPARATOR; ic++)
 -			*oc++ = *ic;
 -		if (*input == '\0')
 +		for (; *ic && *ic != COMMAND_SEPARATOR; ic++) {
 +			if (pos >= sizeof(input) - 1) {
 +				printf("command too long; truncated at %s\n",
 +				       ic);
 +				break;
 +			}
 +			input[pos++] = *ic;
 +		}
 +		if (pos == 0)
  			continue;
  		/* Strip out any trailing spaces */
 -		oc--;
 -		for (; *oc == ' ' && oc > input; oc--);
 -		*++oc = '\0';
 +		pos--;
 +		for (; input[pos] == ' ' && pos > 0; pos--);
 +		input[++pos] = '\0';
  		if (*ic == COMMAND_SEPARATOR)
  			ic++;
  		/* Stop silly command strings like ;;; */
 -		if (*input != '\0')
 +		if (input[0] != '\0')
  			docommand(input);
  		/* Skip leading spaces */
  		for (; *ic == ' '; ic++);


 -- 
 David A. Holland
 dholland@netbsd.org

Home	PR Database Search