[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Mon, 10 Oct 2022 13:11:01 -0700
From: enh <enh@...gle.com>
To: libc-coord@...ts.openwall.com
Subject: Re: fgets behavior for n<=0 (and =1)
On Mon, Oct 10, 2022 at 12:51 PM Rich Felker <dalias@...c.org> wrote:
> 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"..?
>
well, my point was that no _android_ applications are using n==0 :-)
> 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>
> <(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
> > >
>
Content of type "text/html" skipped
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.
