On Thu, Mar 15, 2018 at 04:00:37PM -0300, dgutson . wrote:
> On Thu, Mar 15, 2018 at 3:53 PM, Rich Felker <dalias@libc.org> wrote:
>
> > On Thu, Mar 15, 2018 at 03:48:32PM -0300, Martin Galvan wrote:
> > > 2018-03-15 15:39 GMT-03:00 Rich Felker <dalias@libc.org>:
> > > >> (e.g. the FD* issue reported by Martin Galvan).
> > > >
> > > > That's not a bug. It's compiler warnings being wrongly produced for a
> > > > system header, probably because someone added -I/usr/include or
> > > > similar (normally GCC suppresses these).
> > >
> > > I'm certain we didn't add -I/usr/include or something similar. Could
> > > you test this yourself to confirm it's not a bug?
> >
> > In any case it's not a bug in musl. The code is perfectly valid C. If
> > the compiler is producing a warning for it, either ignore it or ask
> > the compiler to stop.
> >
> > > The compiler warnings aren't being wrongly produced. musl will indeed
> > > perform a signed-to-unsigned conversion here.
> >
> > Because that's how the C language works.
> >
>
> it is a potential vulnerability:
> https://cwe.mitre.org/data/definitions/195.html
> https://wiki.sei.cmu.edu/confluence/display/c/INT31-C.+Ensure+that+integer+conversions+do+not+result+in+lost+or+misinterpreted+data
> https://wiki.sei.cmu.edu/confluence/display/c/INT30-C.+Ensure+that+unsigned+integer+operations+do+not+wrap
>
> Can you ensure it is rocksolid and the signed integer will NEVER be a
> negative value?

FD_* have undefined behavior if the argument is outside the range of
FD_SETSIZE. We could trap this (and if you use fortify headers, they
do) but doing so breaks applications that wrongly allocate larger
space for fd_set buffers for the sake of intentionally using larger fd
values than are possible with the select API.

The behavior of the code with or without the cast to unsigned added is
_exactly the same_. There is no bug here that is fixed by the proposed
patch. The warning is telling you that, if you don't understand how

integer promotions work in C, the code might not do what you expected
it to do. The fact that you seem to think adding a cast "fixes a bug"

is demonstrating how harmful the whole cult around compiler warnings
is: it's not about using them as hints to check your code and make
sure it's doing what you want, but instead about making the warning go
away without actually changing anything.

Rich