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.

4 files changed, 42 insertions, 35 deletions

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);