-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
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.
- --
Stephen Gallagher
RHCE 804006346421761
Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Fedora -
http://enigmail.mozdev.org/
iEYEARECAAYFAku5yxwACgkQeiVVYja6o6PxGACfegxcu3pFNmYRoumURsxU2EkZ
FBkAoKaiQEXoxK2ZdddUHhkiPiWCos1W
=dkQD
-----END PGP SIGNATURE-----