diff --git a/TODO b/TODO index 9e4e07da5ef..d8e13e5e618 100644 --- a/TODO +++ b/TODO @@ -23,6 +23,9 @@ Janitorial Clean-ups: Features: +* maybe add a seccomp-based high-level filter that blocks creation of suid/sgid + files. + * make MAINPID= message reception checks even stricter: if service uses User=, then check sending UID and ignore message if it doesn't match the user or root. diff --git a/src/basic/path-util.c b/src/basic/path-util.c index 221517303ec..1fa813b747d 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -1103,48 +1103,40 @@ int path_simplify_and_warn( unsigned line, const char *lvalue) { - bool absolute, fatal = flag & PATH_CHECK_FATAL; + bool fatal = flag & PATH_CHECK_FATAL; assert(!FLAGS_SET(flag, PATH_CHECK_ABSOLUTE | PATH_CHECK_RELATIVE)); - if (!utf8_is_valid(path)) { - log_syntax_invalid_utf8(unit, LOG_ERR, filename, line, path); - return -EINVAL; - } + if (!utf8_is_valid(path)) + return log_syntax_invalid_utf8(unit, LOG_ERR, filename, line, path); if (flag & (PATH_CHECK_ABSOLUTE | PATH_CHECK_RELATIVE)) { + bool absolute; + absolute = path_is_absolute(path); - if (!absolute && (flag & PATH_CHECK_ABSOLUTE)) { - log_syntax(unit, LOG_ERR, filename, line, 0, - "%s= path is not absolute%s: %s", - lvalue, fatal ? "" : ", ignoring", path); - return -EINVAL; - } + if (!absolute && (flag & PATH_CHECK_ABSOLUTE)) + return log_syntax(unit, LOG_ERR, filename, line, SYNTHETIC_ERRNO(EINVAL), + "%s= path is not absolute%s: %s", + lvalue, fatal ? "" : ", ignoring", path); - if (absolute && (flag & PATH_CHECK_RELATIVE)) { - log_syntax(unit, LOG_ERR, filename, line, 0, - "%s= path is absolute%s: %s", - lvalue, fatal ? "" : ", ignoring", path); - return -EINVAL; - } + if (absolute && (flag & PATH_CHECK_RELATIVE)) + return log_syntax(unit, LOG_ERR, filename, line, SYNTHETIC_ERRNO(EINVAL), + "%s= path is absolute%s: %s", + lvalue, fatal ? "" : ", ignoring", path); } path_simplify(path, true); - if (!path_is_normalized(path)) { - log_syntax(unit, LOG_ERR, filename, line, 0, - "%s= path is not normalized%s: %s", - lvalue, fatal ? "" : ", ignoring", path); - return -EINVAL; - } + if (!path_is_valid(path)) + return log_syntax(unit, LOG_ERR, filename, line, SYNTHETIC_ERRNO(EINVAL), + "%s= path has invalid length (%zu bytes)%s.", + lvalue, strlen(path), fatal ? "" : ", ignoring"); - if (!path_is_valid(path)) { - log_syntax(unit, LOG_ERR, filename, line, 0, - "%s= path has invalid length (%zu bytes)%s.", - lvalue, strlen(path), fatal ? "" : ", ignoring"); - return -EINVAL; - } + if (!path_is_normalized(path)) + return log_syntax(unit, LOG_ERR, filename, line, SYNTHETIC_ERRNO(EINVAL), + "%s= path is not normalized%s: %s", + lvalue, fatal ? "" : ", ignoring", path); return 0; } diff --git a/src/basic/user-util.c b/src/basic/user-util.c index 260f3d20576..a479590e479 100644 --- a/src/basic/user-util.c +++ b/src/basic/user-util.c @@ -80,7 +80,7 @@ char* getlogname_malloc(void) { char *getusername_malloc(void) { const char *e; - e = getenv("USER"); + e = secure_getenv("USER"); if (e) return strdup(e); @@ -238,14 +238,21 @@ int get_user_creds( } if (home) { - if (FLAGS_SET(flags, USER_CREDS_CLEAN) && empty_or_root(p->pw_dir)) - *home = NULL; + if (FLAGS_SET(flags, USER_CREDS_CLEAN) && + (empty_or_root(p->pw_dir) || + !path_is_valid(p->pw_dir) || + !path_is_absolute(p->pw_dir))) + *home = NULL; /* Note: we don't insist on normalized paths, since there are setups that have /./ in the path */ else *home = p->pw_dir; } if (shell) { - if (FLAGS_SET(flags, USER_CREDS_CLEAN) && (isempty(p->pw_shell) || is_nologin_shell(p->pw_shell))) + if (FLAGS_SET(flags, USER_CREDS_CLEAN) && + (isempty(p->pw_shell) || + !path_is_valid(p->pw_dir) || + !path_is_absolute(p->pw_shell) || + is_nologin_shell(p->pw_shell))) *shell = NULL; else *shell = p->pw_shell; @@ -343,6 +350,9 @@ char* uid_to_name(uid_t uid) { if (r != ERANGE) break; + if (bufsize > LONG_MAX/2) /* overflow check */ + return NULL; + bufsize *= 2; } } @@ -384,6 +394,9 @@ char* gid_to_name(gid_t gid) { if (r != ERANGE) break; + if (bufsize > LONG_MAX/2) /* overflow check */ + return NULL; + bufsize *= 2; } } @@ -445,12 +458,12 @@ int get_home_dir(char **_h) { /* Take the user specified one */ e = secure_getenv("HOME"); - if (e && path_is_absolute(e)) { + if (e && path_is_valid(e) && path_is_absolute(e)) { h = strdup(e); if (!h) return -ENOMEM; - *_h = h; + *_h = path_simplify(h, true); return 0; } @@ -480,14 +493,15 @@ int get_home_dir(char **_h) { if (!p) return errno > 0 ? -errno : -ESRCH; - if (!path_is_absolute(p->pw_dir)) + if (!path_is_valid(p->pw_dir) || + !path_is_absolute(p->pw_dir)) return -EINVAL; h = strdup(p->pw_dir); if (!h) return -ENOMEM; - *_h = h; + *_h = path_simplify(h, true); return 0; } @@ -500,13 +514,13 @@ int get_shell(char **_s) { assert(_s); /* Take the user specified one */ - e = getenv("SHELL"); - if (e) { + e = secure_getenv("SHELL"); + if (e && path_is_valid(e) && path_is_absolute(e)) { s = strdup(e); if (!s) return -ENOMEM; - *_s = s; + *_s = path_simplify(s, true); return 0; } @@ -536,14 +550,15 @@ int get_shell(char **_s) { if (!p) return errno > 0 ? -errno : -ESRCH; - if (!path_is_absolute(p->pw_shell)) + if (!path_is_valid(p->pw_shell) || + !path_is_absolute(p->pw_shell)) return -EINVAL; s = strdup(p->pw_shell); if (!s) return -ENOMEM; - *_s = s; + *_s = path_simplify(s, true); return 0; } diff --git a/src/core/execute.c b/src/core/execute.c index c6fd82bbf3e..fb7564b9fe9 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -1697,6 +1697,8 @@ static int build_environment( x = strappend("HOME=", home); if (!x) return -ENOMEM; + + path_simplify(x + 5, true); our_env[n_env++] = x; } @@ -1716,6 +1718,8 @@ static int build_environment( x = strappend("SHELL=", shell); if (!x) return -ENOMEM; + + path_simplify(x + 6, true); our_env[n_env++] = x; } @@ -2738,12 +2742,6 @@ static int acquire_home(const ExecContext *c, uid_t uid, const char** home, char if (!c->working_directory_home) return 0; - if (uid == 0) { - /* Hardcode /root as home directory for UID 0 */ - *home = "/root"; - return 1; - } - r = get_home_dir(buf); if (r < 0) return r; diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index 13e7d3bcdaf..a3b8f43346c 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -2711,7 +2711,6 @@ static int method_can_reboot_to_boot_loader_menu( Manager *m = userdata; int r; - assert(message); assert(m); diff --git a/src/login/logind-user.c b/src/login/logind-user.c index fe47c78bdb1..2b327cbe199 100644 --- a/src/login/logind-user.c +++ b/src/login/logind-user.c @@ -68,6 +68,8 @@ int user_new(User **ret, if (!u->home) return -ENOMEM; + path_simplify(u->home, true); + if (asprintf(&u->state_file, "/run/systemd/users/"UID_FMT, uid) < 0) return -ENOMEM;