From 45f0d8e103c57e9e5e9d92bba1dc2d50b49806de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 15 Sep 2013 22:26:56 -0400 Subject: [PATCH] Verify validity of session name when received from outside Only ASCII letters and digits are allowed. --- Makefile.am | 14 +++++++++++--- TODO | 3 --- src/login/login-shared.c | 8 ++++++++ src/login/login-shared.h | 3 +++ src/login/logind-dbus.c | 1 + src/login/logind-session.c | 1 + src/login/logind-session.h | 1 + src/login/logind.c | 6 ++++++ src/login/sd-login.c | 12 +++++++----- src/shared/cgroup-util.c | 4 +--- src/shared/def.h | 5 +++++ src/shared/env-util.c | 5 ++--- src/shared/replace-var.c | 3 ++- src/shared/unit-name.c | 5 ++--- 14 files changed, 50 insertions(+), 21 deletions(-) create mode 100644 src/login/login-shared.c create mode 100644 src/login/login-shared.h diff --git a/Makefile.am b/Makefile.am index 7b7539a..6014521 100644 --- a/Makefile.am +++ b/Makefile.am @@ -2324,7 +2324,10 @@ if HAVE_ACL libudev_core_la_SOURCES += \ src/udev/udev-builtin-uaccess.c \ src/login/logind-acl.c \ - src/login/sd-login.c + src/login/sd-login.c \ + src/systemd/sd-login.h \ + src/login/login-shared.c \ + src/login/login-shared.h libudev_core_la_LIBADD += \ libsystemd-acl.la @@ -3759,7 +3762,9 @@ libsystemd_logind_core_la_SOURCES = \ src/login/logind-session-dbus.c \ src/login/logind-seat-dbus.c \ src/login/logind-user-dbus.c \ - src/login/logind-acl.h + src/login/logind-acl.h \ + src/login/login-shared.c \ + src/login/login-shared.h libsystemd_logind_core_la_CFLAGS = \ $(AM_CFLAGS) \ @@ -3860,7 +3865,10 @@ tests += \ test-login-tables libsystemd_login_la_SOURCES = \ - src/login/sd-login.c + src/login/sd-login.c \ + src/systemd/sd-login.h \ + src/login/login-shared.c \ + src/login/login-shared.h libsystemd_login_la_CFLAGS = \ $(AM_CFLAGS) \ diff --git a/TODO b/TODO index 9943b3e..bfeaa81 100644 --- a/TODO +++ b/TODO @@ -142,9 +142,6 @@ Features: * journald: make sure ratelimit is actually really per-service with the new cgroup changes -* libsystemd-logind: sd_session_is_active() and friends: verify - validity of session name before appending it to a path - * gparted needs to disable auto-activation of mount units somehow, or maybe we should stop doing auto-activation of this after boot entirely. https://bugzilla.gnome.org/show_bug.cgi?id=701676 diff --git a/src/login/login-shared.c b/src/login/login-shared.c new file mode 100644 index 0000000..ff13c28 --- /dev/null +++ b/src/login/login-shared.c @@ -0,0 +1,8 @@ +#include "login-shared.h" +#include "def.h" + +bool session_id_valid(const char *id) { + assert(id); + + return id + strspn(id, LETTERS DIGITS) == '\0'; +} diff --git a/src/login/login-shared.h b/src/login/login-shared.h new file mode 100644 index 0000000..728ef00 --- /dev/null +++ b/src/login/login-shared.h @@ -0,0 +1,3 @@ +#include + +bool session_id_valid(const char *id); diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index 345df9f..d052e74 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -554,6 +554,7 @@ static int bus_manager_create_session(Manager *m, DBusMessage *message) { * the audit data and let's better register a new * ID */ if (hashmap_get(m->sessions, id)) { + log_warning("Existing logind session ID %s used by new audit session, ignoring", id); audit_id = 0; free(id); diff --git a/src/login/logind-session.c b/src/login/logind-session.c index a726fb1..2d22a68 100644 --- a/src/login/logind-session.c +++ b/src/login/logind-session.c @@ -41,6 +41,7 @@ Session* session_new(Manager *m, const char *id) { assert(m); assert(id); + assert(session_id_valid(id)); s = new0(Session, 1); if (!s) diff --git a/src/login/logind-session.h b/src/login/logind-session.h index edaae8d..9cf6485 100644 --- a/src/login/logind-session.h +++ b/src/login/logind-session.h @@ -29,6 +29,7 @@ typedef enum KillWho KillWho; #include "logind.h" #include "logind-seat.h" #include "logind-user.h" +#include "login-shared.h" typedef enum SessionState { SESSION_OPENING, /* Session scope is being created */ diff --git a/src/login/logind.c b/src/login/logind.c index 9094567..4ef92b8 100644 --- a/src/login/logind.c +++ b/src/login/logind.c @@ -684,6 +684,12 @@ int manager_enumerate_sessions(Manager *m) { if (!dirent_is_file(de)) continue; + if (!session_id_valid(de->d_name)) { + log_warning("Invalid session file name '%s', ignoring.", de->d_name); + r = -EINVAL; + continue; + } + k = manager_add_session(m, de->d_name, &s); if (k < 0) { log_error("Failed to add session by file name %s: %s", de->d_name, strerror(-k)); diff --git a/src/login/sd-login.c b/src/login/sd-login.c index 8a7838d..71d8c29 100644 --- a/src/login/sd-login.c +++ b/src/login/sd-login.c @@ -31,6 +31,7 @@ #include "sd-login.h" #include "strv.h" #include "fileio.h" +#include "login-shared.h" _public_ int sd_pid_get_session(pid_t pid, char **session) { if (pid < 0) @@ -226,17 +227,19 @@ static int file_of_session(const char *session, char **_p) { assert(_p); - if (session) + if (session) { + if (!session_id_valid(session)) + return -EINVAL; + p = strappend("/run/systemd/sessions/", session); - else { - char *buf; + } else { + _cleanup_free_ char *buf = NULL; r = sd_pid_get_session(0, &buf); if (r < 0) return r; p = strappend("/run/systemd/sessions/", buf); - free(buf); } if (!p) @@ -255,7 +258,6 @@ _public_ int sd_session_is_active(const char *session) { return r; r = parse_env_file(p, NEWLINE, "ACTIVE", &s, NULL); - if (r < 0) return r; diff --git a/src/shared/cgroup-util.c b/src/shared/cgroup-util.c index 1d545e0..0bffebd 100644 --- a/src/shared/cgroup-util.c +++ b/src/shared/cgroup-util.c @@ -1511,9 +1511,7 @@ char *cg_unescape(const char *p) { } #define CONTROLLER_VALID \ - "0123456789" \ - "abcdefghijklmnopqrstuvwxyz" \ - "ABCDEFGHIJKLMNOPQRSTUVWXYZ" \ + DIGITS LETTERS \ "_" bool cg_controller_is_valid(const char *p, bool allow_named) { diff --git a/src/shared/def.h b/src/shared/def.h index 5abb544..edd0bcf 100644 --- a/src/shared/def.h +++ b/src/shared/def.h @@ -33,3 +33,8 @@ #define SIGNALS_CRASH_HANDLER SIGSEGV,SIGILL,SIGFPE,SIGBUS,SIGQUIT,SIGABRT #define SIGNALS_IGNORE SIGPIPE + +#define DIGITS "0123456789" +#define LOWERCASE_LETTERS "abcdefghijklmnopqrstuvwxyz" +#define UPPERCASE_LETTERS "ABCDEFGHIJKLMNOPQRSTUVWXYZ" +#define LETTERS LOWERCASE_LETTERS UPPERCASE_LETTERS diff --git a/src/shared/env-util.c b/src/shared/env-util.c index 6a52fb9..5e29629 100644 --- a/src/shared/env-util.c +++ b/src/shared/env-util.c @@ -27,11 +27,10 @@ #include "utf8.h" #include "util.h" #include "env-util.h" +#include "def.h" #define VALID_CHARS_ENV_NAME \ - "0123456789" \ - "abcdefghijklmnopqrstuvwxyz" \ - "ABCDEFGHIJKLMNOPQRSTUVWXYZ" \ + DIGITS LETTERS \ "_" #ifndef ARG_MAX diff --git a/src/shared/replace-var.c b/src/shared/replace-var.c index e11c57a..478fc43 100644 --- a/src/shared/replace-var.c +++ b/src/shared/replace-var.c @@ -24,6 +24,7 @@ #include "macro.h" #include "util.h" #include "replace-var.h" +#include "def.h" /* * Generic infrastructure for replacing @FOO@ style variables in @@ -40,7 +41,7 @@ static int get_variable(const char *b, char **r) { if (*b != '@') return 0; - k = strspn(b + 1, "ABCDEFGHIJKLMNOPQRSTUVWXYZ_"); + k = strspn(b + 1, UPPERCASE_LETTERS "_"); if (k <= 0 || b[k+1] != '@') return 0; diff --git a/src/shared/unit-name.c b/src/shared/unit-name.c index 1baa6eb..8f6c28e 100644 --- a/src/shared/unit-name.c +++ b/src/shared/unit-name.c @@ -26,11 +26,10 @@ #include "path-util.h" #include "util.h" #include "unit-name.h" +#include "def.h" #define VALID_CHARS \ - "0123456789" \ - "abcdefghijklmnopqrstuvwxyz" \ - "ABCDEFGHIJKLMNOPQRSTUVWXYZ" \ + DIGITS LETTERS \ ":-_.\\" static const char* const unit_type_table[_UNIT_TYPE_MAX] = {