From 496d277a64957ba4c9af8a4ffab9f9ed5eaa54e4 Mon Sep 17 00:00:00 2001 From: Dave Love Date: Sun, 14 May 2017 22:26:30 +0100 Subject: [PATCH 15/30] Check for gid 0 ownership as well as uid 0 --- src/lib/mount/home/home.c | 3 ++- src/lib/ns/user/user.c | 4 ++-- src/lib/sessiondir.c | 2 +- src/sexec.c | 6 +++--- src/util/file.c | 5 +++-- src/util/file.h | 2 +- 6 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/lib/mount/home/home.c b/src/lib/mount/home/home.c index 409f40a3..224106f2 100644 --- a/src/lib/mount/home/home.c +++ b/src/lib/mount/home/home.c @@ -47,6 +47,7 @@ int singularity_mount_home(void) { char *sessiondir = singularity_sessiondir_get(); struct passwd *pw; uid_t uid = singularity_priv_getuid(); + gid_t gid = singularity_priv_getgid(); singularity_config_rewind(); if ( singularity_config_get_bool("mount home", 1) <= 0 ) { @@ -135,7 +136,7 @@ int singularity_mount_home(void) { // Check to make sure whatever we were given as the home directory is really ours singularity_message(DEBUG, "Checking permissions on home directory: %s\n", homedir_source); - if ( is_owner(homedir_source, uid) < 0 ) { + if ( is_owner(homedir_source, uid, gid) < 0 ) { singularity_message(ERROR, "Home directory ownership incorrect: %s\n", homedir_source); ABORT(255); } diff --git a/src/lib/ns/user/user.c b/src/lib/ns/user/user.c index f9f3110d..88a359ff 100644 --- a/src/lib/ns/user/user.c +++ b/src/lib/ns/user/user.c @@ -48,7 +48,7 @@ int singularity_ns_user_enabled(void) { // Check to make sure we are SUID or error void check_for_suid(void) { - if ( ( is_owner("/proc/self/exe", 0) != 0 ) || ( is_suid("/proc/self/exe") < 0 ) ) { + if ( ( is_owner("/proc/self/exe", 0, 0) != 0 ) || ( is_suid("/proc/self/exe") < 0 ) ) { singularity_message(ERROR, "User namespace not supported, and program not running privileged.\n"); ABORT(255); } @@ -57,7 +57,7 @@ void check_for_suid(void) { int singularity_ns_user_unshare(void) { - if ( ( is_suid("/proc/self/exe") == 0 ) && ( is_owner("/proc/self/exe", 0) == 0) ) { + if ( ( is_suid("/proc/self/exe") == 0 ) && ( is_owner("/proc/self/exe", 0, 0) == 0) ) { singularity_message(VERBOSE, "Not virtualizing user namespace: running SUID root\n"); return(0); } diff --git a/src/lib/sessiondir.c b/src/lib/sessiondir.c index 1e5a9047..d1d2821b 100644 --- a/src/lib/sessiondir.c +++ b/src/lib/sessiondir.c @@ -90,7 +90,7 @@ char *singularity_sessiondir_init(char *file) { } } - if ( is_owner(sessiondir, singularity_priv_getuid()) < 0 ) { + if ( is_owner(sessiondir, singularity_priv_getuid(), singularity_priv_getgid()) < 0 ) { singularity_message(ERROR, "Session directory has wrong ownership: %s\n", sessiondir); ABORT(255); } diff --git a/src/sexec.c b/src/sexec.c index 075d21b3..1d0b7b69 100644 --- a/src/sexec.c +++ b/src/sexec.c @@ -46,12 +46,12 @@ int main(int argc, char **argv) { singularity_message(VERBOSE2, "Running SUID program workflow\n"); singularity_message(VERBOSE2, "Checking program has appropriate permissions\n"); - if ( ( getuid() != 0 ) && ( ( is_owner("/proc/self/exe", 0) < 0 ) || ( is_suid("/proc/self/exe") < 0 ) ) ) { + if ( ( getuid() != 0 ) && ( ( is_owner("/proc/self/exe", 0, 0) < 0 ) || ( is_suid("/proc/self/exe") < 0 ) ) ) { singularity_abort(255, "This program must be SUID root\n"); } singularity_message(VERBOSE2, "Checking configuration file is properly owned by root\n"); - if ( is_owner(joinpath(SYSCONFDIR, "/singularity/singularity.conf"), 0 ) < 0 ) { + if ( is_owner(joinpath(SYSCONFDIR, "/singularity/singularity.conf"), 0, 0 ) < 0 ) { singularity_abort(255, "Running in privileged mode, root must own the Singularity configuration file\n"); } @@ -91,7 +91,7 @@ int main(int argc, char **argv) { singularity_message(VERBOSE, "Checking for sexec-suid at %s\n", sexec_suid_path); if ( is_file(sexec_suid_path) == 0 ) { - if ( ( is_owner(sexec_suid_path, 0 ) == 0 ) && ( is_suid(sexec_suid_path) == 0 ) ) { + if ( ( is_owner(sexec_suid_path, 0, 0 ) == 0 ) && ( is_suid(sexec_suid_path) == 0 ) ) { singularity_message(VERBOSE, "Invoking SUID sexec: %s\n", sexec_suid_path); execv(sexec_suid_path, argv); // Flawfinder: ignore diff --git a/src/util/file.c b/src/util/file.c index a47eb9b1..67e262e3 100644 --- a/src/util/file.c +++ b/src/util/file.c @@ -174,7 +174,8 @@ int is_write(char *path) { return(-1); } -int is_owner(char *path, uid_t uid) { +int is_owner(char *path, uid_t uid, gid_t gid) { + /* fixme: check up the directory tree? */ struct stat filestat; // Stat path @@ -182,7 +183,7 @@ int is_owner(char *path, uid_t uid) { return(-1); } - if ( uid == filestat.st_uid ) { + if ( uid == filestat.st_uid && gid == filestat.st_gid ) { return(0); } diff --git a/src/util/file.h b/src/util/file.h index 8c0033c5..9bde94a7 100644 --- a/src/util/file.h +++ b/src/util/file.h @@ -27,7 +27,7 @@ int is_dir(char *path); int is_exec(char *path); int is_write(char *path); int is_suid(char *path); -int is_owner(char *path, uid_t uid); +int is_owner(char *path, uid_t uid, gid_t gid); int is_blk(char *path); int is_chr(char *path); int s_mkpath(char *dir, mode_t mode); -- 2.11.0