picnoir/Systemd
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

util: when using basename() for creating temporary files, verify the resulting name is actually valid

Browse source 
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.
This commit is contained in:
Lennart Poettering 2014-12-12 02:32:33 +01:00
parent 8d1c8bd746
commit ae6c3cc009
11 changed files with 67 additions and 60 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
src/core/load-fragment.c
View file

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

2
src/hostname/hostnamed.c
View file

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

12
src/journal/coredump.c
View file

@ -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;
}

2
src/journal/journald-native.c
View file

@ -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;
}

4
src/locale/localed.c
View file

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

2
src/shared/dropin.c
View file

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

2
src/shared/locale-util.c
View file

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

67
src/shared/util.c
View file

@ -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;
return -ENOMEM;
fn = basename(p);
k = fn - p;
strcpy(stpcpy(stpcpy(mempcpy(t, p, fn - p), "."), fn), "XXXXXX");
strcpy(stpcpy(stpcpy(mempcpy(t, p, k), "."), 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;
return -ENOMEM;
fn = basename(p);
k = fn - p;
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" */

6
src/shared/util.h
View file

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

8
src/systemctl/systemctl.c
View file

@ -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) {

20
src/test/test-util.c
View file

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