summaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorLennart Poettering <lennart@poettering.net>2014-12-12 02:32:33 +0100
committerLennart Poettering <lennart@poettering.net>2014-12-12 13:35:32 +0100
commitae6c3cc009a21df4b51851fb8fe3fde0b7d6d8f0 (patch)
tree98eda04d3ff6c24177217efbe9613ed6ed2dc51b /src
parent8d1c8bd746a6a14dec7470f93f843bcb0699f4b8 (diff)
util: when using basename() for creating temporary files, verify the resulting name is actually valid
Also, rename filename_is_safe() to filename_is_valid(), since it actually does a full validation for what the kernel will accept as file name, it's not just a heuristic.
Diffstat (limited to 'src')
-rw-r--r--src/core/load-fragment.c2
-rw-r--r--src/hostname/hostnamed.c2
-rw-r--r--src/journal/coredump.c12
-rw-r--r--src/journal/journald-native.c2
-rw-r--r--src/locale/localed.c4
-rw-r--r--src/shared/dropin.c2
-rw-r--r--src/shared/locale-util.c2
-rw-r--r--src/shared/util.c67
-rw-r--r--src/shared/util.h6
-rw-r--r--src/systemctl/systemctl.c8
-rw-r--r--src/test/test-util.c20
11 files changed, 67 insertions, 60 deletions
diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c
index 259323bd5c..3fbe680cf0 100644
--- a/src/core/load-fragment.c
+++ b/src/core/load-fragment.c
@@ -3066,7 +3066,7 @@ int config_parse_runtime_directory(
if (!n)
return log_oom();
- if (!filename_is_safe(n)) {
+ if (!filename_is_valid(n)) {
log_syntax(unit, LOG_ERR, filename, line, EINVAL,
"Runtime directory is not valid, ignoring assignment: %s", rvalue);
continue;
diff --git a/src/hostname/hostnamed.c b/src/hostname/hostnamed.c
index 970e800769..ef45e563c8 100644
--- a/src/hostname/hostnamed.c
+++ b/src/hostname/hostnamed.c
@@ -552,7 +552,7 @@ static int set_machine_info(Context *c, sd_bus *bus, sd_bus_message *m, int prop
/* The icon name might ultimately be used as file
* name, so better be safe than sorry */
- if (prop == PROP_ICON_NAME && !filename_is_safe(name))
+ if (prop == PROP_ICON_NAME && !filename_is_valid(name))
return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid icon name '%s'", name);
if (prop == PROP_PRETTY_HOSTNAME && string_has_cc(name, NULL))
return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid pretty host name '%s'", name);
diff --git a/src/journal/coredump.c b/src/journal/coredump.c
index be45a684e5..8678ec6a59 100644
--- a/src/journal/coredump.c
+++ b/src/journal/coredump.c
@@ -306,9 +306,9 @@ static int save_external_coredump(
if (r < 0)
return log_error_errno(r, "Failed to determine coredump file name: %m");
- tmp = tempfn_random(fn);
- if (!tmp)
- return log_oom();
+ r = tempfn_random(fn, &tmp);
+ if (r < 0)
+ return log_error_errno(r, "Failed to determine temporary file name: %m");
mkdir_p_label("/var/lib/systemd/coredump", 0755);
@@ -352,9 +352,9 @@ static int save_external_coredump(
goto uncompressed;
}
- tmp_compressed = tempfn_random(fn_compressed);
- if (!tmp_compressed) {
- log_oom();
+ r = tempfn_random(fn_compressed, &tmp_compressed);
+ if (r < 0) {
+ log_error_errno(r, "Failed to determine temporary file name for %s: %m", fn_compressed);
goto uncompressed;
}
diff --git a/src/journal/journald-native.c b/src/journal/journald-native.c
index f982696255..f701233bbe 100644
--- a/src/journal/journald-native.c
+++ b/src/journal/journald-native.c
@@ -350,7 +350,7 @@ void server_process_native_file(
return;
}
- if (!filename_is_safe(e)) {
+ if (!filename_is_valid(e)) {
log_error("Received file in subdirectory of allowed directories. Refusing.");
return;
}
diff --git a/src/locale/localed.c b/src/locale/localed.c
index 8d60d0ff13..0aaa63de85 100644
--- a/src/locale/localed.c
+++ b/src/locale/localed.c
@@ -965,8 +965,8 @@ static int method_set_vc_keyboard(sd_bus *bus, sd_bus_message *m, void *userdata
if (!streq_ptr(keymap, c->vc_keymap) ||
!streq_ptr(keymap_toggle, c->vc_keymap_toggle)) {
- if ((keymap && (!filename_is_safe(keymap) || !string_is_safe(keymap))) ||
- (keymap_toggle && (!filename_is_safe(keymap_toggle) || !string_is_safe(keymap_toggle))))
+ if ((keymap && (!filename_is_valid(keymap) || !string_is_safe(keymap))) ||
+ (keymap_toggle && (!filename_is_valid(keymap_toggle) || !string_is_safe(keymap_toggle))))
return sd_bus_error_set_errnof(error, -EINVAL, "Received invalid keymap data");
r = bus_verify_polkit_async(m, CAP_SYS_ADMIN, "org.freedesktop.locale1.set-keyboard", interactive, &c->polkit_registry, error);
diff --git a/src/shared/dropin.c b/src/shared/dropin.c
index ac09be984a..c5c4f95719 100644
--- a/src/shared/dropin.c
+++ b/src/shared/dropin.c
@@ -43,7 +43,7 @@ int drop_in_file(const char *dir, const char *unit, unsigned level,
if (!b)
return -ENOMEM;
- if (!filename_is_safe(b))
+ if (!filename_is_valid(b))
return -EINVAL;
p = strjoin(dir, "/", unit, ".d", NULL);
diff --git a/src/shared/locale-util.c b/src/shared/locale-util.c
index 9addb05f09..61db9a8125 100644
--- a/src/shared/locale-util.c
+++ b/src/shared/locale-util.c
@@ -195,7 +195,7 @@ bool locale_is_valid(const char *name) {
if (!utf8_is_valid(name))
return false;
- if (!filename_is_safe(name))
+ if (!filename_is_valid(name))
return false;
if (!string_is_safe(name))
diff --git a/src/shared/util.c b/src/shared/util.c
index 6383c0f80a..273552f622 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -4284,15 +4284,15 @@ int fd_wait_for_event(int fd, int event, usec_t t) {
int fopen_temporary(const char *path, FILE **_f, char **_temp_path) {
FILE *f;
char *t;
- int fd;
+ int r, fd;
assert(path);
assert(_f);
assert(_temp_path);
- t = tempfn_xxxxxx(path);
- if (!t)
- return -ENOMEM;
+ r = tempfn_xxxxxx(path, &t);
+ if (r < 0)
+ return r;
fd = mkostemp_safe(t, O_WRONLY|O_CLOEXEC);
if (fd < 0) {
@@ -4403,13 +4403,14 @@ int vt_disallocate(const char *name) {
int symlink_atomic(const char *from, const char *to) {
_cleanup_free_ char *t = NULL;
+ int r;
assert(from);
assert(to);
- t = tempfn_random(to);
- if (!t)
- return -ENOMEM;
+ r = tempfn_random(to, &t);
+ if (r < 0)
+ return r;
if (symlink(from, t) < 0)
return -errno;
@@ -4424,12 +4425,13 @@ int symlink_atomic(const char *from, const char *to) {
int mknod_atomic(const char *path, mode_t mode, dev_t dev) {
_cleanup_free_ char *t = NULL;
+ int r;
assert(path);
- t = tempfn_random(path);
- if (!t)
- return -ENOMEM;
+ r = tempfn_random(path, &t);
+ if (r < 0)
+ return r;
if (mknod(t, mode, dev) < 0)
return -errno;
@@ -4444,12 +4446,13 @@ int mknod_atomic(const char *path, mode_t mode, dev_t dev) {
int mkfifo_atomic(const char *path, mode_t mode) {
_cleanup_free_ char *t = NULL;
+ int r;
assert(path);
- t = tempfn_random(path);
- if (!t)
- return -ENOMEM;
+ r = tempfn_random(path, &t);
+ if (r < 0)
+ return r;
if (mkfifo(t, mode) < 0)
return -errno;
@@ -5561,7 +5564,7 @@ int get_shell(char **_s) {
return 0;
}
-bool filename_is_safe(const char *p) {
+bool filename_is_valid(const char *p) {
if (isempty(p))
return false;
@@ -6963,42 +6966,45 @@ int fflush_and_check(FILE *f) {
return 0;
}
-char *tempfn_xxxxxx(const char *p) {
+int tempfn_xxxxxx(const char *p, char **ret) {
const char *fn;
char *t;
- size_t k;
assert(p);
+ assert(ret);
+
+ fn = basename(p);
+ if (!filename_is_valid(fn))
+ return -EINVAL;
t = new(char, strlen(p) + 1 + 6 + 1);
if (!t)
- return NULL;
-
- fn = basename(p);
- k = fn - p;
+ return -ENOMEM;
- strcpy(stpcpy(stpcpy(mempcpy(t, p, k), "."), fn), "XXXXXX");
+ strcpy(stpcpy(stpcpy(mempcpy(t, p, fn - p), "."), fn), "XXXXXX");
- return t;
+ *ret = t;
+ return 0;
}
-char *tempfn_random(const char *p) {
+int tempfn_random(const char *p, char **ret) {
const char *fn;
char *t, *x;
uint64_t u;
- size_t k;
unsigned i;
assert(p);
+ assert(ret);
+
+ fn = basename(p);
+ if (!filename_is_valid(fn))
+ return -EINVAL;
t = new(char, strlen(p) + 1 + 16 + 1);
if (!t)
- return NULL;
-
- fn = basename(p);
- k = fn - p;
+ return -ENOMEM;
- x = stpcpy(stpcpy(mempcpy(t, p, k), "."), fn);
+ x = stpcpy(stpcpy(mempcpy(t, p, fn - p), "."), fn);
u = random_u64();
for (i = 0; i < 16; i++) {
@@ -7008,7 +7014,8 @@ char *tempfn_random(const char *p) {
*x = 0;
- return t;
+ *ret = t;
+ return 0;
}
/* make sure the hostname is not "localhost" */
diff --git a/src/shared/util.h b/src/shared/util.h
index 73bd9012fd..a15ce95a65 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -714,7 +714,7 @@ _alloc_(2, 3) static inline void *memdup_multiply(const void *p, size_t a, size_
return memdup(p, a * b);
}
-bool filename_is_safe(const char *p) _pure_;
+bool filename_is_valid(const char *p) _pure_;
bool path_is_safe(const char *p) _pure_;
bool string_is_safe(const char *p) _pure_;
bool string_has_cc(const char *p, const char *ok) _pure_;
@@ -1015,8 +1015,8 @@ int bind_remount_recursive(const char *prefix, bool ro);
int fflush_and_check(FILE *f);
-char *tempfn_xxxxxx(const char *p);
-char *tempfn_random(const char *p);
+int tempfn_xxxxxx(const char *p, char **ret);
+int tempfn_random(const char *p, char **ret);
bool is_localhost(const char *hostname);
diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index 2ceb303f1b..5ed430c82f 100644
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -5730,16 +5730,16 @@ static int unit_file_find_path(LookupPaths *lp, const char *unit_name, char **un
}
static int create_edit_temp_file(const char *new_path, const char *original_path, char **ret_tmp_fn) {
- int r;
char *t;
+ int r;
assert(new_path);
assert(original_path);
assert(ret_tmp_fn);
- t = tempfn_random(new_path);
- if (!t)
- return log_oom();
+ r = tempfn_random(new_path, &t);
+ if (r < 0)
+ return log_error_errno(r, "Failed to determine temporary filename for %s: %m", new_path);
r = mkdir_parents(new_path, 0755);
if (r < 0) {
diff --git a/src/test/test-util.c b/src/test/test-util.c
index fe54586eee..6c7d77b19b 100644
--- a/src/test/test-util.c
+++ b/src/test/test-util.c
@@ -802,24 +802,24 @@ static void test_foreach_string(void) {
assert_se(streq(x, "zzz"));
}
-static void test_filename_is_safe(void) {
+static void test_filename_is_valid(void) {
char foo[FILENAME_MAX+2];
int i;
- assert_se(!filename_is_safe(""));
- assert_se(!filename_is_safe("/bar/foo"));
- assert_se(!filename_is_safe("/"));
- assert_se(!filename_is_safe("."));
- assert_se(!filename_is_safe(".."));
+ assert_se(!filename_is_valid(""));
+ assert_se(!filename_is_valid("/bar/foo"));
+ assert_se(!filename_is_valid("/"));
+ assert_se(!filename_is_valid("."));
+ assert_se(!filename_is_valid(".."));
for (i=0; i<FILENAME_MAX+1; i++)
foo[i] = 'a';
foo[FILENAME_MAX+1] = '\0';
- assert_se(!filename_is_safe(foo));
+ assert_se(!filename_is_valid(foo));
- assert_se(filename_is_safe("foo_bar-333"));
- assert_se(filename_is_safe("o.o"));
+ assert_se(filename_is_valid("foo_bar-333"));
+ assert_se(filename_is_valid("o.o"));
}
static void test_string_has_cc(void) {
@@ -1360,7 +1360,7 @@ int main(int argc, char *argv[]) {
test_hexdump();
test_log2i();
test_foreach_string();
- test_filename_is_safe();
+ test_filename_is_valid();
test_string_has_cc();
test_ascii_strlower();
test_files_same();