GNU bug report logs - #11108
chmod: fix symlink race condition

Previous Next

Full log

Message #8 received at 11108 <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 11108 <at> debbugs.gnu.org
Subject: Re: bug#11108: [PATCH] chmod: fix symlink race condition
Date: Wed, 28 Mar 2012 09:36:01 +0200

Paul Eggert wrote:
> This fixes what I hope is an obvious race condition
> that can occur if some other process substitutes a
> symlink for a non-symlink while chmod is running.

Good catch.  I'll bet that's exploitable by anyone who
can convince root to run "chmod -r ... DIR" on files they own.

The chmodat-introducing commit was v5.92-656-gc97a36e,
but the preceding use of chmod was just as vulnerable.
If you reference a commit in your log, please use "git describe"
output, not the bare-8-byte-SHA1 like we've done in the past.
While "git describe" output is not converted to a clickable link
by a released gitk, with the one in upcoming git-1.7.10, it is.

I presume you'll update NEWS, too, where you can say
[bug introduced in the beginning]
I've confirmed that the very first version of chmod.c has the same
problem: it calls stat, then calls chmod whenever !S_ISLNK.

I note also that this doesn't protect anyone who is using
a system that lacks both fchmodat and lchmod.
For that, we'd have to openat each file to get a file descriptor,
then fstat that FD to verify it's the same dev/ino as
found by the fts-run stat call, and only then, call fchmod.

> =====
> * src/chmod.c (process_file): Don't follow symlink if we
> think the file is not a symlink.
> ---
>  src/chmod.c |   10 +++++++++-
>  1 files changed, 9 insertions(+), 1 deletions(-)
>
> diff --git a/src/chmod.c b/src/chmod.c
> index aa4ac77..2e1f1c7 100644
> --- a/src/chmod.c
> +++ b/src/chmod.c
> @@ -268,7 +268,15 @@ process_file (FTS *fts, FTSENT *ent)
>
>        if (! S_ISLNK (old_mode))
>          {
> -          if (chmodat (fts->fts_cwd_fd, file, new_mode) == 0)
> +          /* Use any native support for AT_SYMLINK_NOFOLLOW, to avoid
> +             following a symlink if there is a race.  */
> +          #if HAVE_FCHMODAT || HAVE_LCHMOD
> +          int follow_flag = AT_SYMLINK_NOFOLLOW;
> +          #else
> +          int follow_flag = 0;
> +          #endif
> +
> +          if (fchmodat (fts->fts_cwd_fd, file, new_mode, follow_flag) == 0)
>              chmod_succeeded = true;
>            else
>              {

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.