[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sat, 24 Oct 2015 17:36:45 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: [PATCH] Fix off-by-one buffer overflow in getdelim
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
View attachment "getdelim.c" of type "text/plain" (423 bytes)
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
