From owner-freebsd-bugs@FreeBSD.ORG Thu Mar 6 10:01:30 2008 Return-Path: Delivered-To: freebsd-bugs@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id DAD55106566B; Thu, 6 Mar 2008 10:01:30 +0000 (UTC) (envelope-from rwatson@FreeBSD.org) Received: from cyrus.watson.org (cyrus.watson.org [209.31.154.42]) by mx1.freebsd.org (Postfix) with ESMTP id 44D508FC25; Thu, 6 Mar 2008 10:01:29 +0000 (UTC) (envelope-from rwatson@FreeBSD.org) Received: from fledge.watson.org (fledge.watson.org [209.31.154.41]) by cyrus.watson.org (Postfix) with ESMTP id 0956446B4D; Thu, 6 Mar 2008 05:01:28 -0500 (EST) Date: Thu, 6 Mar 2008 10:01:27 +0000 (GMT) From: Robert Watson X-X-Sender: robert@fledge.watson.org To: Michael Plass In-Reply-To: <20071227224807.40C8F1702A@shuttle.plass-family.net> Message-ID: <20080306095345.S12811@fledge.watson.org> References: <20071227224807.40C8F1702A@shuttle.plass-family.net> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: freebsd-bugs@FreeBSD.org, FreeBSD-gnats-submit@FreeBSD.org Subject: Re: kern/119079: [patch] DDB input routine reads/writes beyond end of buffer X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 06 Mar 2008 10:01:31 -0000 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@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-bugs > To unsubscribe, send any mail to "freebsd-bugs-unsubscribe@freebsd.org" >