[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#12632: file permissions checking mishandled when setuid
From: |
Paul Eggert |
Subject: |
bug#12632: file permissions checking mishandled when setuid |
Date: |
Sun, 14 Oct 2012 11:14:39 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux i686; rv:16.0) Gecko/20121011 Thunderbird/16.0.1 |
On 10/13/2012 11:56 PM, Eli Zaretskii wrote:
> This won't compile on Windows, since there's no 'euidaccess' (yet).
Thanks, that should be easy enough to fix, so I gave it a whirl
(patch below).
> Emacs should be able to test whether a file exists even if it
> will be unable to access it later.
Emacs cannot do that. What 'access' does is ask, "If Emacs were
to issue the seteuid system call, and change the effective user
ID to the real user ID, would Emacs then be able to see that the
file exists?" This does not test whether the file exists; it tests
only whether Emacs could see that the file exists in a hypothetical
situation that never actually happens (because Emacs never issues
the seteuid system call). But this isn't what is wanted here:
what is wanted is a test whether Emacs can currently see that the
file exists, and that is what euidaccess does.
> In any case, using 'euidaccess' here subtly changes the semantics of
> file-exists-p
Currently file-exists-p uses 'stat', and 'euidaccess' uses
the same access check that 'stat' does -- they both use the
effective uid. So the semantics aren't changing here.
>> > 'dir' can't be nil there.
> file-name-directory can return nil
It's an absolute file name, so file-name-directory can't return nil.
(This point is moot now, since the patch doesn't remove the
unnecessary NILP check.)
Here's the incremental patch to add euidaccess to Windows.
I'm attaching the entire revised patch, relative to trunk
bzr 110544.
=== modified file 'nt/ChangeLog'
--- nt/ChangeLog 2012-10-08 14:14:22 +0000
+++ nt/ChangeLog 2012-10-14 18:06:01 +0000
@@ -1,3 +1,9 @@
+2012-10-14 Paul Eggert <eggert@cs.ucla.edu>
+
+ Use euidaccess, not access, when checking file permissions (Bug#12632).
+ * inc/ms-w32.h (euidaccess): Define this, not 'access', since
+ the mainline code now uses euidaccess.
+
2012-10-08 Juanma Barranquero <lekktu@gmail.com>
* config.nt: Sync with autogen/config.in.
=== modified file 'nt/inc/ms-w32.h'
--- nt/inc/ms-w32.h 2012-09-30 21:36:42 +0000
+++ nt/inc/ms-w32.h 2012-10-14 18:06:01 +0000
@@ -160,8 +160,6 @@
#endif
/* Calls that are emulated or shadowed. */
-#undef access
-#define access sys_access
#undef chdir
#define chdir sys_chdir
#undef chmod
@@ -176,6 +174,7 @@
#define dup sys_dup
#undef dup2
#define dup2 sys_dup2
+#define euidaccess sys_access
#define fopen sys_fopen
#define link sys_link
#define localtime sys_localtime
=== modified file 'src/ChangeLog'
--- src/ChangeLog 2012-10-14 06:02:52 +0000
+++ src/ChangeLog 2012-10-14 18:06:01 +0000
@@ -5,14 +5,13 @@
(LIBES): Use it.
* callproc.c (init_callproc):
* charset.c (init_charset):
- * fileio.c (check_existing) [!DOS_NT]:
- (check_executable) [!DOS_NT && !HAVE_EUIDACCESS]:
+ * fileio.c (check_existing, check_executable):
* lread.c (openp, load_path_check):
* process.c (allocate_pty):
* xrdb.c (file_p):
Use euidaccess, not access. Use symbolic names instead of integers
for the flags, as they're portable now.
- * fileio.c (Ffile_readable_p) [!DOS_NT && !macintosh]:
+ * fileio.c (Ffile_readable_p):
Use euidaccess, not stat + open + close.
(file_directory_p): New function, which uses 'stat' on most places
but 'access' (for efficiency) if WINDOWSNT.
@@ -21,6 +20,7 @@
* lread.c (openp): When opening a file, use fstat rather than
stat, as that avoids a permissions race. When not opening a file,
use file_directory_p rather than stat.
+ * msdos.c (init_environment, readlink): Use sys_access, not access.
2012-10-13 Jan Djärv <jan.h.d@swipnet.se>
=== modified file 'src/fileio.c'
--- src/fileio.c 2012-10-14 06:02:52 +0000
+++ src/fileio.c 2012-10-14 18:06:01 +0000
@@ -2424,16 +2424,7 @@
bool
check_existing (const char *filename)
{
-#ifdef DOS_NT
- /* The full emulation of Posix 'stat' is too expensive on
- DOS/Windows, when all we want to know is whether the file exists.
- So we use 'access' instead, which is much more lightweight. */
- /* FIXME: an euidaccess implementation should be added to the
- DOS/Windows ports and this #ifdef branch should be removed. */
- return (access (filename, F_OK) >= 0);
-#else
return euidaccess (filename, F_OK) == 0;
-#endif
}
/* Return true if file FILENAME exists and can be executed. */
@@ -2441,16 +2432,7 @@
static bool
check_executable (char *filename)
{
-#ifdef DOS_NT
- /* FIXME: an euidaccess implementation should be added to the
- DOS/Windows ports and this #ifdef branch should be removed. */
- struct stat st;
- if (stat (filename, &st) < 0)
- return 0;
- return ((st.st_mode & S_IEXEC) != 0);
-#else /* not DOS_NT */
return euidaccess (filename, X_OK) == 0;
-#endif /* not DOS_NT */
}
/* Return true if file FILENAME exists and can be written. */
@@ -2546,16 +2528,7 @@
return call2 (handler, Qfile_readable_p, absname);
absname = ENCODE_FILE (absname);
-
-#if defined (DOS_NT) || defined (macintosh)
- /* FIXME: an euidaccess implementation should be added to the
- DOS/Windows ports and this #ifdef branch should be removed. */
- if (access (SDATA (absname), 0) == 0)
- return Qt;
- return Qnil;
-#else /* not DOS_NT and not macintosh */
return euidaccess (SSDATA (absname), R_OK) == 0 ? Qt : Qnil;
-#endif /* not DOS_NT and not macintosh */
}
/* Having this before file-symlink-p mysteriously caused it to be forgotten
@@ -2592,7 +2565,7 @@
/* The read-only attribute of the parent directory doesn't affect
whether a file or directory can be created within it. Some day we
should check ACLs though, which do affect this. */
- return (access (SDATA (dir), D_OK) < 0) ? Qnil : Qt;
+ return file_directory_p (SDATA (dir)) ? Qt : Qnil;
#else
return (check_writable (!NILP (dir) ? SSDATA (dir) : "")
? Qt : Qnil);
@@ -2694,8 +2667,8 @@
file_directory_p (char const *file)
{
#ifdef WINDOWSNT
- /* 'access' is cheaper than 'stat'. */
- return access (file, D_OK) == 0;
+ /* This is cheaper than 'stat'. */
+ return euidaccess (file, D_OK) == 0;
#else
struct stat st;
return stat (file, &st) == 0 && S_ISDIR (st.st_mode);
=== modified file 'src/msdos.c'
--- src/msdos.c 2012-09-23 08:44:20 +0000
+++ src/msdos.c 2012-10-14 18:06:01 +0000
@@ -3557,7 +3557,7 @@
read-only filesystem, like CD-ROM or a write-protected floppy.
The only way to be really sure is to actually create a file and
see if it succeeds. But I think that's too much to ask. */
- if (tmp && access (tmp, D_OK) == 0)
+ if (tmp && sys_access (tmp, D_OK) == 0)
{
setenv ("TMPDIR", tmp, 1);
break;
@@ -3935,7 +3935,7 @@
readlink (const char *name, char *dummy1, size_t dummy2)
{
/* `access' is much faster than `stat' on MS-DOS. */
- if (access (name, F_OK) == 0)
+ if (sys_access (name, F_OK) == 0)
errno = EINVAL;
return -1;
}
euidaccess.txt
Description: Text document
- bug#12632: file permissions checking mishandled when setuid, Paul Eggert, 2012/10/12
- bug#12632: file permissions checking mishandled when setuid, Eli Zaretskii, 2012/10/13
- bug#12632: file permissions checking mishandled when setuid, Eli Zaretskii, 2012/10/13
- bug#12632: file permissions checking mishandled when setuid, Paul Eggert, 2012/10/14
- bug#12632: file permissions checking mishandled when setuid, Eli Zaretskii, 2012/10/14
- bug#12632: file permissions checking mishandled when setuid,
Paul Eggert <=
- bug#12632: file permissions checking mishandled when setuid, Eli Zaretskii, 2012/10/14
- bug#12632: file permissions checking mishandled when setuid, Paul Eggert, 2012/10/14
- bug#12632: file permissions checking mishandled when setuid, Eli Zaretskii, 2012/10/14
- bug#12632: file permissions checking mishandled when setuid, Eli Zaretskii, 2012/10/14
- bug#12632: file permissions checking mishandled when setuid, Paul Eggert, 2012/10/14
- bug#12632: file permissions checking mishandled when setuid, Eli Zaretskii, 2012/10/14
- bug#12632: file permissions checking mishandled when setuid, Paul Eggert, 2012/10/15
- bug#12632: file permissions checking mishandled when setuid, Eli Zaretskii, 2012/10/15
- bug#12632: file permissions checking mishandled when setuid, Paul Eggert, 2012/10/15
- bug#12632: file permissions checking mishandled when setuid, Eli Zaretskii, 2012/10/15