[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 14 Feb 2018 20:45:56 +0100
From: Markus Wichmann <nullplan@....net>
To: musl@...ts.openwall.com
Subject: Re: fread() - possible division by zero
On Wed, Feb 14, 2018 at 04:50:44PM -0200, Geraldo Netto wrote:
> Dear Friends,
>
> It seems we may have the same division by zero issue on fread():
>
> This is the original code:
>
> size_t fread(void *restrict destv, size_t size, size_t nmemb, FILE *restrict f)
> {
> unsigned char *dest = destv;
> size_t len = size*nmemb, l = len, k;
> if (!size) nmemb = 0;
>
> FLOCK(f);
>
> f->mode |= f->mode-1;
>
> if (f->rend - f->rpos > 0) {
> /* First exhaust the buffer. */
> k = MIN(f->rend - f->rpos, l);
> memcpy(dest, f->rpos, k);
> f->rpos += k;
> dest += k;
> l -= k;
> }
>
> /* Read the remainder directly */
> for (; l; l-=k, dest+=k) {
> k = __toread(f) ? 0 : f->read(f, dest, l);
> if (k+1<=1) {
> FUNLOCK(f);
> return (len-l)/size;
> }
> }
>
> FUNLOCK(f);
> return nmemb;
> }
>
>
>
>
> It seems we need to check the variable size on return because if size is
> zero
> We'll have a division by zero and a segmentation fault
>
If size is zero, then l is zero. Even with a filled read buffer, that
can't change since l is lower than all possible other unsigned values,
thus k will be zero as well. So at the "for" loop, l will be zero, thus
the loop will never be entered, thus the return statement is unreachable
in this case. So again, no division will take place, least of all one by
zero.
Say, did you even test these cases? It would take all of five minutes to
do, and belay your suspicions quicker than I can write these responses.
HTH,
Markus
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.
