From 0d7468cabdb40152d7715356c4768d6459c119da Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Tue, 27 Dec 2005 07:55:50 +0000 Subject: [PATCH] (defined_S_IFMT): New macro. Include stat-macros.h. Include stdlib.h, for abort(). Don't include stdio.h or assert.h; no longer needed. (same_file_type): Don't assume S_IFMT is defined, as POSIX does not require this. Don't assume S_IFCHR and S_IFBLK have their usual sort of bit pattern. (fchmod_new): Open with O_NOCTTY for as well, for minor improvement on hosts where that matters. Don't bother to assert, since the caller (in this source file) checks the same thing. Discard any errno from a close failure, for consistency with other code. --- lib/chmod-safer.c | 77 ++++++++++++++++++++++++++--------------------- 1 file changed, 43 insertions(+), 34 deletions(-) diff --git a/lib/chmod-safer.c b/lib/chmod-safer.c index aac61dfc8..3d0ec64f6 100644 --- a/lib/chmod-safer.c +++ b/lib/chmod-safer.c @@ -23,16 +23,21 @@ #include "chmod-safer.h" +#ifdef S_IFMT +# define defined_S_IFMT true +#else +# define defined_S_IFMT false +#endif + +#include "stat-macros.h" + #include -#include -#include +#include #include #include #include -#include "fcntl--.h" /* for the open->open_safer mapping */ - -#if !defined O_NOFOLLOW +#ifndef O_NOFOLLOW # define O_NOFOLLOW 0 #endif @@ -48,12 +53,17 @@ static inline bool same_file_type (struct stat const *st, dev_t device, mode_t type) { /* The types must always match. */ - if ( ! (st->st_mode & type)) + if (! (defined_S_IFMT ? (st->st_mode & S_IFMT) == type + : type == S_IFDIR ? S_ISDIR (st->st_mode) + : type == S_IFIFO ? S_ISFIFO (st->st_mode) + : type == S_IFBLK ? S_ISBLK (st->st_mode) + : type == S_IFCHR ? S_ISCHR (st->st_mode) + : (abort (), false))) return false; /* For character and block devices, the major and minor device numbers must match, too. */ - if (type & (S_IFCHR | S_IFBLK)) + if (S_ISBLK (type) || S_ISCHR (type)) return st->st_rdev == device; return true; @@ -66,40 +76,39 @@ same_file_type (struct stat const *st, dev_t device, mode_t type) static int fchmod_new (char const *file, mode_t mode, dev_t device, mode_t file_type) { - int fail = 1; + int result = 0; struct stat sb; - int saved_errno = 0; - int fd = open (file, O_NOFOLLOW | O_RDONLY | O_NDELAY); + int fd = open (file, O_RDONLY | O_NOCTTY | O_NOFOLLOW | O_NONBLOCK); + int saved_errno; - assert (O_NOFOLLOW); + if (fd < 0) + return -1; - if (0 <= fd - && fstat (fd, &sb) == 0 - /* Given the entry we've just created, if its link count is - not 1 or its type/device has changed, then someone may be - 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 fchmod and set errno, so that the - code below reports the failure to set permissions. - Note that we don't check the link count if the expected - type is `directory'. */ - && (((sb.st_nlink == 1 || file_type == S_IFDIR) - && same_file_type (&sb, device, file_type)) - || ((errno = EACCES), 0)) - && fchmod (fd, mode) == 0) + /* Given the entry we've just created, if its link count is + not 1 or its type/device has changed, then someone may be + 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 fchmod and set errno, so that the + code below reports the failure to set permissions. + Note that we don't check the link count if the expected + type is `directory'. */ + result = fstat (fd, &sb); + if (result == 0) { - fail = 0; - } - else - { - saved_errno = errno; + if ((sb.st_nlink == 1 || S_ISDIR (file_type)) + && same_file_type (&sb, device, file_type)) + result = fchmod (fd, mode); + else + { + errno = EACCES; + result = -1; + } } - if (0 <= fd && close (fd) != 0 && saved_errno == 0) - saved_errno = errno; - + saved_errno = errno; + close (fd); errno = saved_errno; - return fail; + return result; } /* Use a safer variant of chmod, if the underlying system facilities permit.