[SSSD] [PATCH] Protect against check-and-open race conditions

Stephen Gallagher sgallagh at redhat.com
Mon Apr 5 12:29:28 UTC 2010


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04/05/2010 08:13 AM, Dmitri Pal wrote:
> 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.
>>
> I might be missing something but I do not get it.
> If you open the file first and then do fstat() on the file descriptor
> you do not care if it is a symlink or not.
> You care that the file you opened has the right mode and ownership and
> this is what you are checking.
> And nobody can do anything using inotify() to the file you already opened.
> I do not see a race condition here and I do not see a security issue here.
> Am I missing something?

No, you've got it exactly correct. I'm saying that if you do fstat(),
it's perfectly safe. lstat() is an easy way to guarantee a
race-condition. And lstat() is the only way to be certain that you're
using a symlink. So in short, it's unsafe to care whether a symlink is
in use.

In the older code, we were first checking whether a file was a symlink,
and then opening it if it was a real file. This is an unsafe premise, as
there's no way to guarantee that the file is STILL not a symlink when
the open() call happens. And it's not really sensible to care if a
symlink is in use anyway.

> 
> 
>>
>>> 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.
>>
>>
> 
> -------------------------
> 
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://fedorahosted.org/mailman/listinfo/sssd-devel
> 
> 


- -- 
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/

iEYEARECAAYFAku516gACgkQeiVVYja6o6P/5QCgnGy8Jsjz+NqpAWOCwD/08Gzk
ktYAnjwUGAwmIV8ksyRiqAu5fCpDbDkt
=TYU+
-----END PGP SIGNATURE-----



More information about the sssd-devel mailing list