Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20180214193942.nar6nvuulv4rg5nt@voyager>
Date: Wed, 14 Feb 2018 20:39:42 +0100
From: Markus Wichmann <nullplan@....net>
To: musl@...ts.openwall.com
Subject: Re: fwrite() - possible division by zero

On Wed, Feb 14, 2018 at 04:24:16PM -0200, Geraldo Netto wrote:
> Dear Friends,
> 
> I was playing with musl and I think I may have found an issue on fwrite():
> 
> This is the original code:
> 
> size_t fwrite(const void *restrict src, size_t size, size_t nmemb,
> FILE *restrict f)
> {
>     size_t k, l = size*nmemb;
>     if (!size) nmemb = 0;
>     FLOCK(f);
>     k = __fwritex(src, l, f);
>     FUNLOCK(f);
>     return k==l ? nmemb : k/size;
> }
> 
> 
> 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. So __fwritex will be called with l as
zero. Which means, if you read that code, that it will have to return
zero. So in the end, k will be zero as well, so k==l, so nmemb will be
returned (which was set to zero earlier), and more importantly, no
division takes place.

> I'm sending the attached patch that changes the return as follows:
> 
> return k==l ? nmemb : (size != 0) ? k/size : k;
> 
> 

Also style: Usual style for musl is to write comparisons with zero as
boolean operations, and to use as few parentheses as possible, i.e.

return k==l ? nmemb : size ? k/size : k;

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.