kern/119079: [patch] DDB input routine reads/writes beyond end of buffer

[Robert Watson]

On Thu, 27 Dec 2007, Michael Plass wrote:

>> Description:
> 	The ddb input routine db_readline() includes the terminating newline
> 	and NUL characters in the returned buffer, but it does not take this
> 	into account when checking against the caller-supplied limit.
>> How-To-Repeat:
> 	Enter DDB and type enough characters to fill the buffer
> 	(120 characters).  Hit enter, and then use the up-arrow key to
> 	scroll back through history.  Note that it picks up garbage past the
> 	end of the original line.
>> Fix:
> 	The patch checks the provided lsize and decreases by two to leave
> 	room for the newline and NUL; it also clears these two characters,
> 	because some of the code paths don't provide the terminating NUL.
> 	(The patch also corrects a problem in history redraw when the cursor
> 	is not at the end of the line while scrolling back though history.)

Dear Michael,

Thanks for this report; I'm able to reproduce both problems, and am currently 
looking at your patch.  I'm going to immediately commit the redraw fix, but 
did have one question about the remainder of the patch below.

> @@ -302,6 +302,10 @@
> 	char *	lstart;
> 	int	lsize;
> {
> +	if (lsize < 3)
> +		return (0);
> +	lstart[lsize - 1] = lstart[lsize - 2] = 0;
> +	lsize -= 2;	/* allow space for newline and terminating NUL */

You comment that you need to leave room for two bytes here -- terminating 
newline and nul.  Could you clarify in what cases the NL may be missing?  It 
looks to me like the loop around db_inputchar() effectively ensures that we 
never leave that loop without the buffer containing an NL, and that the NL 
should fit in the buffer.  I concur on the nul termination reading.  So my 
question is: is it not sufficient to simply do the following:

 	lstart[lsize - 1] = 0;
 	lsize--;

Thanks,

Robert N M Watson
Computer Laboratory
University of Cambridge

> 	if (lsize != db_lhistlsize) {
> 		/*
> 		 * (Re)initialize input line history.  Throw away any
> --- db_input_bufoverflow.patch ends here ---
>
>
>> Release-Note:
>> Audit-Trail:
>> Unformatted:
> _______________________________________________
> freebsd-bugs at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-bugs
> To unsubscribe, send any mail to "freebsd-bugs-unsubscribe at freebsd.org"
>