Blob Blame Raw
From 496d277a64957ba4c9af8a4ffab9f9ed5eaa54e4 Mon Sep 17 00:00:00 2001
From: Dave Love <dave.love@manchester.ac.uk>
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