From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/13273 Path: news.gmane.org!.POSTED!not-for-mail From: Rich Felker Newsgroups: gmane.linux.lib.musl.general Subject: Re: [PATCH] Fix off-by-one buffer overflow in getdelim Date: Sun, 16 Sep 2018 14:34:47 -0400 Message-ID: <20180916183447.GC17995@brightrain.aerifal.cx> References: <20151024204339.GA1352@nyan> <20180916182542.GB17995@brightrain.aerifal.cx> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: blaine.gmane.org 1537122777 12012 195.159.176.226 (16 Sep 2018 18:32:57 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Sun, 16 Sep 2018 18:32:57 +0000 (UTC) User-Agent: Mutt/1.5.21 (2010-09-15) Cc: Felix Janda To: musl@lists.openwall.com Original-X-From: musl-return-13289-gllmg-musl=m.gmane.org@lists.openwall.com Sun Sep 16 20:32:53 2018 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by blaine.gmane.org with smtp (Exim 4.84_2) (envelope-from ) id 1g1bqm-00030D-H4 for gllmg-musl@m.gmane.org; Sun, 16 Sep 2018 20:32:52 +0200 Original-Received: (qmail 13998 invoked by uid 550); 16 Sep 2018 18:35:01 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Original-Received: (qmail 13974 invoked from network); 16 Sep 2018 18:35:01 -0000 Content-Disposition: inline In-Reply-To: <20180916182542.GB17995@brightrain.aerifal.cx> Original-Sender: Rich Felker Xref: news.gmane.org gmane.linux.lib.musl.general:13273 Archived-At: On Sun, Sep 16, 2018 at 02:25:42PM -0400, Rich Felker wrote: > On Sat, Oct 24, 2015 at 10:43:39PM +0200, Felix Janda wrote: > > when deciding whether to resize the buffer, the terminating null byte > > was not taken into account > > --- > > src/stdio/getdelim.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/src/stdio/getdelim.c b/src/stdio/getdelim.c > > index a88c393..3077490 100644 > > --- a/src/stdio/getdelim.c > > +++ b/src/stdio/getdelim.c > > @@ -27,7 +27,7 @@ ssize_t getdelim(char **restrict s, size_t *restrict n, int delim, FILE *restric > > for (;;) { > > z = memchr(f->rpos, delim, f->rend - f->rpos); > > k = z ? z - f->rpos + 1 : f->rend - f->rpos; > > - if (i+k >= *n) { > > + if (i+k+1 >= *n) { > > if (k >= SIZE_MAX/2-i) goto oom; > > *n = i+k+2; > > if (*n < SIZE_MAX/4) *n *= 2; > > -- > > 2.4.9 > > This patch raised a potential conformance issue, that by a strict > reading of the spec, getdelim is only permitted to realloc if the > caller-provided buffer length is insufficient: > > "If *lineptr is a null pointer or if the object pointed to by > *lineptr is of insufficient size, an object shall be allocated as > if by malloc() or the object shall be reallocated as if by > realloc(), respectively, ..." > > I'm going to change the +1 to +!z and add a comment. The idea is that > the +1 was only needed in order for the result to fit if the delimiter > has not already been found; if the memchr found it, an exact-sized > buffer was being expanded unnecessarily. > > I'm replying to this thread and CC'ing in case there are any problems > I'm missing in my new fix. This fix actually looks insufficient; it doesn't fix the case where the getc produces EOF rather than a character. Rich