From bc60954fae5eb9c041b0f7dd19296737610da8ce Mon Sep 17 00:00:00 2001 From: Nick Kralevich Date: Sat, 31 Jan 2015 21:39:46 -0800 Subject: [PATCH] builtins.c: Don't require file open() for chmod/chown 42a9349dc4e98019d27d7f8d19bc6c431695d7e1 modified init's builtin chmod, chown, and mkdir calls to avoid following symlinks. This addressed a number of attacks we were seeing at the time where poorly written init scripts were following attacker supplied symlinks resulting in rooting vulnerabilities. To avoid race conditions, the previous implementation only ran fchown / fchmod on file descriptors opened with open(O_NOFOLLOW). Unfortunately, unlike the normal "chown" or "chmod" calls, this requires read or write access to the underlying file. This isn't ideal, as opening some files may have side effects, or init may not have permission to open certain files (such as when SELinux is enabled). Instead of using open(O_NOFOLLOW) + fchown(), use lchown() instead. As before, the target of the symlink won't be modified by chown. This also supports setting the ownership of symlinks. Instead of using open(O_NOFOLLOW) + fchmod(), use fchmodat(AT_SYMLINK_NOFOLLOW) instead. As before, the target of the symlink won't be modified by chmod. This change will continue to ensure that chown/chmod/mkdir doesn't follow symlinks, without requiring init to open every file in read-only or read-write mode. This change depends on bionic commit I1eba0cdb2c509d9193ceecf28f13118188a3cfa7 Addresses the following mako/occam SELinux denial: audit(1422770408.951:6): avc: denied { write } for pid=1 comm="init" name="smd7" dev="tmpfs" ino=7207 scontext=u:r:init:s0 tcontext=u:object_r:radio_device:s0 tclass=chr_file Change-Id: I14fde956784d65c44e7aa91dd7eea9a004df3081 --- init/builtins.c | 71 ++++++------------------------------------------- 1 file changed, 8 insertions(+), 63 deletions(-) diff --git a/init/builtins.c b/init/builtins.c index d42ec8114..76c0a1887 100644 --- a/init/builtins.c +++ b/init/builtins.c @@ -49,6 +49,8 @@ #include +#define chmod DO_NOT_USE_CHMOD_USE_FCHMODAT_SYMLINK_NOFOLLOW + int add_environment(const char *name, const char *value); extern int init_module(void *, unsigned long, const char *); @@ -76,63 +78,6 @@ static int write_file(const char *path, const char *value) } } -static int _open(const char *path) -{ - int fd; - - fd = open(path, O_RDONLY | O_NOFOLLOW); - if (fd < 0) - fd = open(path, O_WRONLY | O_NOFOLLOW); - - return fd; -} - -static int _chown(const char *path, unsigned int uid, unsigned int gid) -{ - int fd; - int ret; - - fd = _open(path); - if (fd < 0) { - return -1; - } - - ret = fchown(fd, uid, gid); - if (ret < 0) { - int errno_copy = errno; - close(fd); - errno = errno_copy; - return -1; - } - - close(fd); - - return 0; -} - -static int _chmod(const char *path, mode_t mode) -{ - int fd; - int ret; - - fd = _open(path); - if (fd < 0) { - return -1; - } - - ret = fchmod(fd, mode); - if (ret < 0) { - int errno_copy = errno; - close(fd); - errno = errno_copy; - return -1; - } - - close(fd); - - return 0; -} - static int insmod(const char *filename, char *options) { void *module; @@ -320,7 +265,7 @@ int do_mkdir(int nargs, char **args) ret = make_dir(args[1], mode); /* chmod in case the directory already exists */ if (ret == -1 && errno == EEXIST) { - ret = _chmod(args[1], mode); + ret = fchmodat(AT_FDCWD, args[1], mode, AT_SYMLINK_NOFOLLOW); } if (ret == -1) { return -errno; @@ -334,13 +279,13 @@ int do_mkdir(int nargs, char **args) gid = decode_uid(args[4]); } - if (_chown(args[1], uid, gid) < 0) { + if (lchown(args[1], uid, gid) == -1) { return -errno; } /* chown may have cleared S_ISUID and S_ISGID, chmod again */ if (mode & (S_ISUID | S_ISGID)) { - ret = _chmod(args[1], mode); + ret = fchmodat(AT_FDCWD, args[1], mode, AT_SYMLINK_NOFOLLOW); if (ret == -1) { return -errno; } @@ -814,10 +759,10 @@ out: int do_chown(int nargs, char **args) { /* GID is optional. */ if (nargs == 3) { - if (_chown(args[2], decode_uid(args[1]), -1) < 0) + if (lchown(args[2], decode_uid(args[1]), -1) == -1) return -errno; } else if (nargs == 4) { - if (_chown(args[3], decode_uid(args[1]), decode_uid(args[2])) < 0) + if (lchown(args[3], decode_uid(args[1]), decode_uid(args[2])) == -1) return -errno; } else { return -1; @@ -840,7 +785,7 @@ static mode_t get_mode(const char *s) { int do_chmod(int nargs, char **args) { mode_t mode = get_mode(args[1]); - if (_chmod(args[2], mode) < 0) { + if (fchmodat(AT_FDCWD, args[2], mode, AT_SYMLINK_NOFOLLOW) < 0) { return -errno; } return 0;