[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: bug in chdir-safer
From: |
Jim Meyering |
Subject: |
Re: bug in chdir-safer |
Date: |
Sun, 25 Dec 2005 18:42:02 +0100 |
Eric Blake <address@hidden> wrote: >> if
(open_dereferences_symlink >> && (lstat (dir, &sb_init) != 0
|| ! S_ISDIR (sb_init.st_mode))) >> return fail; > > Don't you
want to set errno=ELOOP here? Good catch. Otherwise, errno
wouldn't be usefully defined in that case. However, it's cleaner
simply to remove that S_ISDIR test. 2005-12-25 Jim Meyering
<address@hidden>
* chdir-safer.c (chdir_no_follow): Remove unnecessary
test of S_ISDIR (sb_init.st_mode).
Index: lib/chdir-safer.c
===================================================================
RCS file: /fetish/cu/lib/chdir-safer.c,v
retrieving revision 1.3
retrieving revision 1.4
diff -u -p -u -r1.3 -r1.4
--- lib/chdir-safer.c 23 Dec 2005 12:00:26 -0000 1.3
+++ lib/chdir-safer.c 25 Dec 2005 17:33:57 -0000 1.4
@@ -44,7 +44,10 @@
/* Just like chmod, but fail if DIR is a symbolic link.
This can avoid a minor race condition between when a
- directory is created or stat'd and when we chdir into it. */
+ directory is created or stat'd and when we chdir into it.
+
+ Note that this function fails (while chdir would succeed)
+ if DIR cannot be opened with O_RDONLY. */
int
chdir_no_follow (char const *dir)
{
@@ -56,10 +59,9 @@ chdir_no_follow (char const *dir)
bool open_dereferences_symlink = ! O_NOFOLLOW;
- /* If open follows symlinks, lstat DIR first to ensure that it is
- a directory and to get its device and inode numbers. */
- if (open_dereferences_symlink
- && (lstat (dir, &sb_init) != 0 || ! S_ISDIR (sb_init.st_mode)))
+ /* If open follows symlinks, lstat DIR, to get its device and
+ inode numbers. */
+ if (open_dereferences_symlink && lstat (dir, &sb_init) != 0)
return fail;
fd = open (dir, O_NOFOLLOW | O_RDONLY | O_NDELAY);
@@ -67,11 +69,10 @@ chdir_no_follow (char const *dir)
if (0 <= fd
&& fstat (fd, &sb) == 0
/* If DIR is a different directory, then someone is trying to do
- something nasty. However, the risk of
- such an attack is so low that it isn't worth a special diagnostic.
- Simply skip the fchdir and set errno (to the same value that open
- uses for symlinks with O_NOFOLLOW), so that the caller can
- report the failure. */
+ something nasty. However, the risk of such an attack is so low
+ that it isn't worth a special diagnostic. Simply skip the fchdir
+ and set errno (to the same value that open uses for symlinks with
+ O_NOFOLLOW), so that the caller can report the failure. */
&& ( ! open_dereferences_symlink || SAME_INODE (sb_init, sb)
|| ((errno = ELOOP), 0))
&& fchdir (fd) == 0)
- bug in chdir-safer, Eric Blake, 2005/12/22
- Re: bug in chdir-safer, Eric Blake, 2005/12/22
- Re: bug in chdir-safer, Jim Meyering, 2005/12/23
- Re: bug in chdir-safer, Eric Blake, 2005/12/24
- Re: bug in chdir-safer,
Jim Meyering <=
- Re: bug in chdir-safer, Paul Eggert, 2005/12/25
- Re: bug in chdir-safer, Jim Meyering, 2005/12/26
- Re: bug in chdir-safer, Paul Eggert, 2005/12/26
- Re: bug in chdir-safer, Paul Eggert, 2005/12/27