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

Dmitri Pal dpal at redhat.com
Mon Apr 5 12:13:51 UTC 2010


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?


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


-- 
Thank you,
Dmitri Pal

Engineering Manager IPA project,
Red Hat Inc.


-------------------------------
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/




More information about the sssd-devel mailing list