From 45f0d8e103c57e9e5e9d92bba1dc2d50b49806de Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
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 <stdbool.h>
+
+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] = {