[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20190118203725.GI29911@voyager>
Date: Fri, 18 Jan 2019 21:37:25 +0100
From: Markus Wichmann <nullplan@....net>
To: musl@...ts.openwall.com
Subject: Re: <shadow.h> function: fgetspent_r
Hi all,
so I had a look at the glibc implementation (from v2.24, which I had
lying around), and here are my findings: They don't return on format
error. Instead, they loop.
On buffer too short, they will leave the stream where it is and return
ERANGE. And on end of file, they return ENOENT, which I find bizarre.
Looking at my own implementation, I am not certain if cleaning up a
faulty state is desirable, since non-recoverable situations exist.
fgetspent_r() has the invariant that the file position is pointing to
the start of a line. So, first thought was to use ftell() beforehand and
fseek() on error, to revert to the start of a line. However, that really
only helps with the "buffer too short" case. An illegal line won't
become more acceptable, no matter how many times we reread it.
Rich correctly pointed out that a file might not be seekable. So if
reversing isn't an option, we can only go forward. So I added an fgets()
loop in case the fseek() fails. But fgets() can fail, too. Maybe even
spuriously (what happens if the file is actually an fdopen()ed
nonblocking TCP socket or something?), so that the problem fixes itself.
Then the invariant is still violated and we have no indication of
anything being wrong.
So, since it is impossible to guarantee the invariant, and the user
certainly has no way to check, I think telling the user to close the
file on any error is probably the best choice here (i.e. the choice that
saves me the most work).
Looping on format error sounds attractive as well, but I think we'd want
that behavior to be consistent across the entire src/passwd directory,
right? It would allow slightly broken files to be present in the system
without bricking it. On the other hand, permissivity is usually only a
good thing outside of security contexts.
Thoughts?
Ciao,
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.
