[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Mon, 10 Oct 2022 15:51:33 -0400
From: Rich Felker <dalias@...c.org>
To: libc-coord@...ts.openwall.com
Subject: Re: fgets behavior for n<=0 (and =1)
On Mon, Oct 10, 2022 at 08:50:53AM -0700, enh wrote:
> fwiw, bionic's fgets is just locking plus a call to fgets_unlocked(), which
> has this:
>
> char* fgets_unlocked(char* buf, int n, FILE* fp) {
> if (n <= 0) __fortify_fatal("fgets: buffer size %d <= 0", n);
>
> (we've added a lot of "fortify" stuff for "obviously invalid arguments that
> we've actually seen app bugs from in the wild" over the years.)
This is an argument in favor of being able to interpret it as
undefined, but in absence of it being undefined, this would of course
be nonconforming behavior. I'd be in favor of this for n<0 but I'm not
sure about n=0 -- I wonder if there are any applications intentionally
using n=0. Doesn't seem useful but perhaps it's a cute way to "wait
for the read-mode file no longer to be locked"..?
Rich
> On Fri, Oct 7, 2022 at 4:13 PM Rich Felker <dalias@...c.org> wrote:
>
> > 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 <(214)%20748-3649>
> > 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.
