From 06540c9cef675fd8665325301384f3cc491e9f66 Mon Sep 17 00:00:00 2001 From: Kaz Kylheku Date: Sat, 23 Jul 2022 20:01:15 -0700 Subject: Check using effective UID, not real. We don't want to behave like the access function, which is intended for use in setuid programs to determine what the original user can access. The purpose of safepath_check is to check whether the filesystem can harm the caller. For that, the effective identity that is being wielded should be used. A setuid executable might have a real user ID bob, but effective root. Root does not trust bob; root doesn't want to follow a symlink controlled by bob. * safepath.c (safe_group, tamper_proof): Replace getuid calls with geteuid. * README.md: Updated text. --- README.md | 2 +- safepath.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 3bf9b79..10ef7b5 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@ one or more path components, and returns an indication whether that name is safe for the process to use. Safe means that the pathname doesn't contain any component which could be -tampered with by a user other than the real user ID of the caller, or else +tampered with by a user other than the effective user ID of the caller, or else root. ## What is significance of this check? diff --git a/safepath.c b/safepath.c index 19ecf45..ba4996c 100644 --- a/safepath.c +++ b/safepath.c @@ -55,8 +55,8 @@ static int safe_group(gid_t gid) return 0; } - /* Obtain passwd info about real user ID, to get at the name. */ - if (getpwuid_r(getuid(), &pw_real, buf_real, sizeof buf_real, &pwu) < 0 || + /* Obtain passwd info about effective user ID, to get at the name. */ + if (getpwuid_r(geteuid(), &pw_real, buf_real, sizeof buf_real, &pwu) < 0 || pwu == 0) { return 0; @@ -93,7 +93,7 @@ static int tamper_proof(const struct stat *st) * change the permissions to whatever they want * and modify the object. */ - if (st->st_uid != 0 && st->st_uid != getuid()) + if (st->st_uid != 0 && st->st_uid != geteuid()) return 0; /* Ownership is good, but permissions are open; object is writable to -- cgit v1.2.3