Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 7 Oct 2022 19:13:30 -0400
From: Rich Felker <dalias@...c.org>
To: libc-coord@...ts.openwall.com
Subject: Re: fgets behavior for n<=0 (and =1)

On Fri, Oct 07, 2022 at 01:12:27PM +0000, Pascal Cuoq wrote:
> C/ Have undefined behavior for n=INT_MIN
> 
> Since undefined behavior can be anything and worse, and the C
> standard's description does not explicitly allow “anything and
> worse” for any value of n, this is arguably incorrect.
> 
>       if (n--<=1) {
> https://git.musl-libc.org/cgit/musl/tree/src/stdio/fgets.c

I think we should fix this. In general, doing arithmetic like this on
signed caller-provided values without first range checking is a bug
and we should probably have some kind of linting on that.

With that said, my reading of the spec is that passing n<=0 is
underspecified and arguably UB, if not even required to do something
unsafe and wrong. The spec does not actually use n-1 as a max number
of bytes to write, but a limit on the number of characters to write.
Once that limit is hit, it's supposed to write a null terminator. So
after reading "at most" -1 or "at most" -2147483649 bytes (thus 0
bytes), it clobbers a byte of the buffer you didn't intend it to.

The POSIX text gratuitously differs and says "until n-1 bytes are
read". Since at no point have a negative number of bytes been read,
the terminating condition is never met, and fgets reads forever until
hitting a newline.

This really should be filed as a defect against C, hopefully to get in
a fix for the upcoming standard.

It's ridiculous and frustrating that the POSIX text is gratuitously
different in a way that makes it "more wrong" here, too...

Rich

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.