Re: port-i386/44008: long boot.cfg command overruns buffer

[David Holland <dholland-bugs%netbsd.org@localhost>]

The following reply was made to PR port-i386/44008; it has been noted by GNATS.

From: David Holland <dholland-bugs%netbsd.org@localhost>
To: gnats-bugs%netbsd.org@localhost
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@localhost