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

[Robert Watson]

On Thu, 6 Mar 2008, Michael Plass wrote:

> I was able to reproduce the problem in a version rigged to run in userspace. 
> Check what happens when you just type enough to fill the buffer (or until it 
> beeps) and then hit return.  The normal chars are handled in the default 
> case of the big switch in db_inputchar(), and when the buffer is full (db_le 
> == db_lbuf_end), db_inputchar() returns 0.  The driving loop calls again, 
> this time with the newline, and that gets stored this way:
> 	    case '\n':
> 		/* FALLTHROUGH */
> 	    case '\r':
> 		*db_le++ = c;
> 		return (1); leaving the newline in db_lbuf_end[0] and db_le == 
> db_lbuf_end + 1, so that the terminating nul is stored at db_lbuf_end[1].
>
> I've only lightly tested it, but upon reexamination I think better fix is to 
> leave lsize alone in do_readline() and initialize
>  db_lbuf_end = lstart + lsize - 2; this will assure that each of the lines 
> in the history buffer have proper termination and storing the nul in the 
> initialization won't be needed.

Ah, indeed.  I like the revised fix better also.  I've gone ahead and 
committed the revised patch and will MFC it in a few days.  Thanks for the 
report/fixes!

Robert N M Watson
Computer Laboratory
University of Cambridge

>
> The patch against 1.38 then looks like this:
> --- db_input.c.orig	2008-03-06 22:09:04.000000000 -0800
> +++ db_input.c	2008-03-06 22:25:24.000000000 -0800
> @@ -302,6 +302,8 @@
> 	char *	lstart;
> 	int	lsize;
> {
> +	if (lsize < 2)
> +		return(0);
> 	if (lsize != db_lhistlsize) {
> 		/*
> 		 * (Re)initialize input line history.  Throw away any
> @@ -316,7 +318,7 @@
> 	db_force_whitespace();	/* synch output position */
>
> 	db_lbuf_start = lstart;
> -	db_lbuf_end   = lstart + lsize;
> +	db_lbuf_end   = lstart + lsize - 2;
> 	db_lc = lstart;
> 	db_le = lstart;
>
>
> Thanks for the care you've taken in committing this.
> - Michael
>
>
>