From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/8762 Path: news.gmane.org!not-for-mail From: Rich Felker Newsgroups: gmane.linux.lib.musl.general Subject: Re: [PATCH] Fix off-by-one buffer overflow in getdelim Date: Sat, 24 Oct 2015 17:36:45 -0400 Message-ID: <20151024213645.GU8645@brightrain.aerifal.cx> References: <20151024204339.GA1352@nyan> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="S5HS5MvDw4DmbRmb" X-Trace: ger.gmane.org 1445722629 20071 80.91.229.3 (24 Oct 2015 21:37:09 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sat, 24 Oct 2015 21:37:09 +0000 (UTC) To: musl@lists.openwall.com Original-X-From: musl-return-8775-gllmg-musl=m.gmane.org@lists.openwall.com Sat Oct 24 23:37:04 2015 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by plane.gmane.org with smtp (Exim 4.69) (envelope-from ) id 1Zq6Um-0006Hx-Ou for gllmg-musl@m.gmane.org; Sat, 24 Oct 2015 23:37:00 +0200 Original-Received: (qmail 25614 invoked by uid 550); 24 Oct 2015 21:36:58 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: Original-Received: (qmail 24564 invoked from network); 24 Oct 2015 21:36:58 -0000 Content-Disposition: inline In-Reply-To: <20151024204339.GA1352@nyan> User-Agent: Mutt/1.5.21 (2010-09-15) Original-Sender: Rich Felker Xref: news.gmane.org gmane.linux.lib.musl.general:8762 Archived-At: --S5HS5MvDw4DmbRmb Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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 I think you're mistaken. i+k is the space needed so far in the buffer (not counting the terminating null byte) and *n is the usable space. The equality case of the i+k >= *n conditional covers the need to expand the buffer when the new content of length k would exactly fit but would not leave room for null termination. Just to make sure I wrote a quick test program, which I've attached, that should crash in free if the overflow occurs. It does not crash and the output demonstrates correct resizing. Rich --S5HS5MvDw4DmbRmb Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="getdelim.c" #include #include #include int main() { int i; FILE *f = tmpfile(); putc('\n', f); for (i=1; i<32; i++) { fseek(f, -1, SEEK_END); fputs("x\n", f); rewind(f); ungetc(getc(f), f); size_t n = i+1; char *s = malloc(n);; printf("%zu %zu ", n, malloc_usable_size(s)); size_t ret = getline(&s, &n, f); printf("%zu %zu %zu\n", ret, n, malloc_usable_size(s)); free(s); } } --S5HS5MvDw4DmbRmb--