-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 04/05/2010 01:35 PM, Stephen Gallagher wrote:
On 04/02/2010 06:00 AM, Jakub Hrozek wrote:
> On 04/01/2010 10:20 PM, Stephen Gallagher wrote:
>> There is a small window between running lstat() on a filename and
>> opening it where it's possible for the file to have been modified.
>> We were protecting against this by saving the stat data from the
>> original file and verifying that it was the same file (by device
>> and inode) when we opened it again, but this is an imperfect
>> solution, as it is still possible for an attacker to modify the
>> permissions during this window.
>> It is much better to simply open the file and test on the active
>> file descriptor.
>> Resolves
https://fedorahosted.org/sssd/ticket/425 incidentally, as
>> without the initial lstat, we are implicitly accepting symlinks
>> and only verifying the target file.
> Well, this changes the semantics of the calls a little as it is not
> possible to check for symlinks anymore..granted we don't need it in the
> code right now
Realistically, it's going to be exceptionally rare that we would
actually care whether something was a symlink. If we desperately need to
know this, check_file() can be called individually.
The problem with symlinks is that there's no way to test for them that
is not a race-condition. When you call stat() or fstat(), it's
impossible to determine whether the resulting file being stat()-ed
exists at the original specified location or was resolved through
symlinks. So the only way to determine if a file is a symlink is by
polling it with lstat() before or after doing the stat()/fstat(). In
either case, it's a race-condition, because someone with an inotify()
watch on the file could change it between one stat and the next.
So it's always a security risk to care about whether a file is a
symlink. It's always better to work only with the resulting file.
> As per the code only couple of nits: the DEBUG() call on line 163
> should say "check_fd failed"
Thanks, fixed.
> Why does the check_fd call accept an external stat buf? It is not
> defined in any header file, perhaps it should be made static..did you
> envision exporting it in the future similar to check_file()?
I didn't just envision it, I planned it and then forgot to include it in
the header file :)
Fixed. I also added several comments to the header file to identify the
recommended approach to performing file checks and warn of the potential
race-conditions.
Thank you, ACK, but please remove line 150 which is currently commented
out before pushing.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Fedora -
http://enigmail.mozdev.org/
iEYEARECAAYFAku7KEAACgkQHsardTLnvCUe0ACbBVaV7PhoNd5F036Rf8ZmKhTt
yUwAmwX2CkCWdZ9wiLsmfk1L26pjviqO
=vZsb
-----END PGP SIGNATURE-----