From 592d84e1265d04c3104acee815a503856db503a1 Mon Sep 17 00:00:00 2001 From: Olivier Bal-Petre Date: Tue, 4 Mar 2025 14:37:02 +0100 Subject: [PATCH] pam_namespace: add flags to indicate path safety Add two flags in the script to indicate if the paths to the polydir and the instance directories are safe (root owned and writable by root only). Signed-off-by: Olivier Bal-Petre Signed-off-by: Dmitry V. Levin Upstream-Status: Backport [https://github.com/linux-pam/linux-pam/commit/592d84e1265d04c3104acee815a503856db503a1] CVE: CVE-2025-6020 Signed-off-by: Hitendra Prajapati --- modules/pam_namespace/namespace.init | 56 ++++++++++++------- modules/pam_namespace/pam_namespace.c | 79 ++++++++++++++++++++++++++- 2 files changed, 115 insertions(+), 20 deletions(-) diff --git a/modules/pam_namespace/namespace.init b/modules/pam_namespace/namespace.init index 67d4aa2..8782178 100755 --- a/modules/pam_namespace/namespace.init +++ b/modules/pam_namespace/namespace.init @@ -1,25 +1,43 @@ #!/bin/sh -# It receives polydir path as $1, the instance path as $2, -# a flag whether the instance dir was newly created (0 - no, 1 - yes) in $3, -# and user name in $4. +# It receives as arguments: +# - $1 polydir path (see WARNING below) +# - $2 instance path (see WARNING below) +# - $3 flag whether the instance dir was newly created (0 - no, 1 - yes) +# - $4 user name +# - $5 flag whether the polydir path ($1) is safe (0 - unsafe, 1 -safe) +# - $6 flag whether the instance path ($2) is safe (0 - unsafe, 1 - safe) +# +# WARNING: This script is invoked with full root privileges. Accessing +# the polydir ($1) and the instance ($2) directories in this context may be +# extremely dangerous as those can be under user control. The flags $5 and $6 +# are provided to let you know if all the segments part of the path (except the +# last one) are owned by root and are writable by root only. If the path does +# not meet these criteria, you expose yourself to possible symlink attacks when +# accessing these path. +# However, even if the path components are safe, the content of the +# directories may still be owned/writable by a user, so care must be taken! # # The following section will copy the contents of /etc/skel if this is a # newly created home directory. -if [ "$3" = 1 ]; then - # This line will fix the labeling on all newly created directories - [ -x /sbin/restorecon ] && /sbin/restorecon "$1" - user="$4" - passwd=$(getent passwd "$user") - homedir=$(echo "$passwd" | cut -f6 -d":") - if [ "$1" = "$homedir" ]; then - gid=$(echo "$passwd" | cut -f4 -d":") - cp -rT /etc/skel "$homedir" - chown -R "$user":"$gid" "$homedir" - mask=$(awk '/^UMASK/{gsub("#.*$", "", $2); print $2; exit}' /etc/login.defs) - mode=$(printf "%o" $((0777 & ~$mask))) - chmod ${mode:-700} "$homedir" - [ -x /sbin/restorecon ] && /sbin/restorecon -R "$homedir" - fi -fi +# Executes only if the polydir path is safe +if [ "$5" = 1 ]; then + + if [ "$3" = 1 ]; then + # This line will fix the labeling on all newly created directories + [ -x /sbin/restorecon ] && /sbin/restorecon "$1" + user="$4" + passwd=$(getent passwd "$user") + homedir=$(echo "$passwd" | cut -f6 -d":") + if [ "$1" = "$homedir" ]; then + gid=$(echo "$passwd" | cut -f4 -d":") + cp -rT /etc/skel "$homedir" + chown -R "$user":"$gid" "$homedir" + mask=$(sed -E -n 's/^UMASK[[:space:]]+([^#[:space:]]+).*/\1/p' /etc/login.defs) + mode=$(printf "%o" $((0777 & ~mask))) + chmod ${mode:-700} "$homedir" + [ -x /sbin/restorecon ] && /sbin/restorecon -R "$homedir" + fi + fi +fi exit 0 diff --git a/modules/pam_namespace/pam_namespace.c b/modules/pam_namespace/pam_namespace.c index 22d8445..8cba036 100644 --- a/modules/pam_namespace/pam_namespace.c +++ b/modules/pam_namespace/pam_namespace.c @@ -1390,6 +1390,79 @@ static int check_inst_parent(int dfd, struct instance_data *idata) return PAM_SUCCESS; } +/* + * Check for a given absolute path that all segments except the last one are: + * 1. a directory owned by root and not writable by group or others + * 2. a symlink owned by root and referencing a directory respecting 1. + * Returns 0 if safe, -1 is unsafe. + * If the path is not accessible (does not exist, hidden under a mount...), + * returns -1 (unsafe). + */ +static int check_safe_path(const char *path, struct instance_data *idata) +{ + char *p = strdup(path); + char *d; + char *dir = p; + struct stat st; + + if (p == NULL) + return -1; + + /* Check path is absolute */ + if (p[0] != '/') + goto error; + + strip_trailing_slashes(p); + + /* Last segment of the path may be owned by the user */ + if ((d = strrchr(dir, '/')) != NULL) + *d = '\0'; + + while ((d=strrchr(dir, '/')) != NULL) { + + /* Do not follow symlinks */ + if (lstat(dir, &st) != 0) + goto error; + + if (S_ISLNK(st.st_mode)) { + if (st.st_uid != 0) { + if (idata->flags & PAMNS_DEBUG) + pam_syslog(idata->pamh, LOG_DEBUG, + "Path deemed unsafe: Symlink %s should be owned by root", dir); + goto error; + } + + /* Follow symlinks */ + if (stat(dir, &st) != 0) + goto error; + } + + if (!S_ISDIR(st.st_mode)) { + if (idata->flags & PAMNS_DEBUG) + pam_syslog(idata->pamh, LOG_DEBUG, + "Path deemed unsafe: %s is expected to be a directory", dir); + goto error; + } + + if (st.st_uid != 0 || + ((st.st_mode & (S_IWGRP|S_IWOTH)) && !(st.st_mode & S_ISVTX))) { + if (idata->flags & PAMNS_DEBUG) + pam_syslog(idata->pamh, LOG_DEBUG, + "Path deemed unsafe: %s should be owned by root, and not be writable by group or others", dir); + goto error; + } + + *d = '\0'; + } + + free(p); + return 0; + +error: + free(p); + return -1; +} + /* * Check to see if there is a namespace initialization script in * the /etc/security directory. If such a script exists @@ -1438,7 +1511,11 @@ static int inst_init(const struct polydir_s *polyptr, const char *ipath, close_fds_pre_exec(idata); execle(init_script, init_script, - polyptr->dir, ipath, newdir?"1":"0", idata->user, NULL, envp); + polyptr->dir, ipath, + newdir ? "1":"0", idata->user, + (check_safe_path(polyptr->dir, idata) == -1) ? "0":"1", + (check_safe_path(ipath, idata) == -1) ? "0":"1", + NULL, envp); _exit(1); } else if (pid > 0) { while (((rc = waitpid(pid, &status, 0)) == (pid_t)-1) && -- 2.50.1