From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/13272 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:25:42 -0400 Message-ID: <20180916182542.GB17995@brightrain.aerifal.cx> References: <20151024204339.GA1352@nyan> 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 1537122236 2926 195.159.176.226 (16 Sep 2018 18:23:56 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Sun, 16 Sep 2018 18:23:56 +0000 (UTC) User-Agent: Mutt/1.5.21 (2010-09-15) Cc: Felix Janda To: musl@lists.openwall.com Original-X-From: musl-return-13288-gllmg-musl=m.gmane.org@lists.openwall.com Sun Sep 16 20:23:52 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 1g1bi2-0000dJ-53 for gllmg-musl@m.gmane.org; Sun, 16 Sep 2018 20:23:50 +0200 Original-Received: (qmail 8104 invoked by uid 550); 16 Sep 2018 18:25:58 -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 8080 invoked from network); 16 Sep 2018 18:25:57 -0000 Content-Disposition: inline In-Reply-To: <20151024204339.GA1352@nyan> Original-Sender: Rich Felker Xref: news.gmane.org gmane.linux.lib.musl.general:13272 Archived-At: 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. Rich