diff options
| -rw-r--r-- | util/libreboot-utils/lib/file.c | 13 | ||||
| -rw-r--r-- | util/libreboot-utils/lib/io.c | 4 | ||||
| -rw-r--r-- | util/libreboot-utils/lib/mkhtemp.c | 39 |
3 files changed, 46 insertions, 10 deletions
diff --git a/util/libreboot-utils/lib/file.c b/util/libreboot-utils/lib/file.c index 4623748c..3620f425 100644 --- a/util/libreboot-utils/lib/file.c +++ b/util/libreboot-utils/lib/file.c @@ -691,6 +691,10 @@ rootfs(void) } /* filesystem sandboxing in userspace + * TODO: + missing length bound check. + potential CPU DoS on very long paths, spammed repeatedly. + perhaps cap at PATH_LEN? */ int fs_resolve_at(int dirfd, const char *path, int flags) @@ -754,6 +758,15 @@ err: return -1; } +/* NOTE: + rejects . and .. but not empty strings + after normalisation. edge case: + ////// + + normalised implicitly, but might be good + to add a defensive check regardless. code + probably not exploitable in current state. + */ int fs_next_component(const char **p, char *name, size_t namesz) diff --git a/util/libreboot-utils/lib/io.c b/util/libreboot-utils/lib/io.c index a09e3992..eac6073e 100644 --- a/util/libreboot-utils/lib/io.c +++ b/util/libreboot-utils/lib/io.c @@ -4,6 +4,10 @@ * I/O functions specific to nvmutil. */ +/* TODO: local tmpfiles not being deleted + when flags==O_RDONLY e.g. dump command + */ + #include <sys/types.h> #include <sys/stat.h> diff --git a/util/libreboot-utils/lib/mkhtemp.c b/util/libreboot-utils/lib/mkhtemp.c index b5b9aeeb..61479b25 100644 --- a/util/libreboot-utils/lib/mkhtemp.c +++ b/util/libreboot-utils/lib/mkhtemp.c @@ -206,8 +206,8 @@ env_tmpdir(int bypass_all_sticky_checks, char **tmpdir, int allow_noworld_unsticky; int saved_errno = errno; - char tmp[] = "/tmp"; - char vartmp[] = "/var/tmp"; + static const char tmp[] = "/tmp"; + static const char vartmp[] = "/var/tmp"; /* tmpdir is a user override, if set */ if (override_tmpdir == NULL) @@ -238,26 +238,26 @@ env_tmpdir(int bypass_all_sticky_checks, char **tmpdir, allow_noworld_unsticky = 0; - if (world_writeable_and_sticky("/tmp", + if (world_writeable_and_sticky(tmp, allow_noworld_unsticky, bypass_all_sticky_checks)) { if (tmpdir != NULL) - *tmpdir = tmp; + *tmpdir = (char *)tmp; errno = saved_errno; - return "/tmp"; + return (char *)tmp; } - if (world_writeable_and_sticky("/var/tmp", + if (world_writeable_and_sticky(vartmp, allow_noworld_unsticky, bypass_all_sticky_checks)) { if (tmpdir != NULL) - *tmpdir = vartmp; + *tmpdir = (char *)vartmp; errno = saved_errno; - return "/var/tmp"; + return (char *)vartmp; } return NULL; @@ -327,11 +327,11 @@ same_dir(const char *a, const char *b) if (rval_scmp == 0) goto success_same_dir; - fd_a = fs_open(a, O_RDONLY | O_DIRECTORY); + fd_a = fs_open(a, O_RDONLY | O_DIRECTORY | O_NOFOLLOW); if (fd_a < 0) goto err_same_dir; - fd_b = fs_open(b, O_RDONLY | O_DIRECTORY); + fd_b = fs_open(b, O_RDONLY | O_DIRECTORY | O_NOFOLLOW); if (fd_b < 0) goto err_same_dir; @@ -513,6 +513,17 @@ sticky_hell: * file ownership under hardened mode * (only lets you touch your own files/dirs) */ +/* + TODO: + some variables e.g. template vs suffix, + assumes they match. + we should test this explicitly, + but the way this is called is + currently safe - this would however + be nice for future library use + by outside projects. + this whole code needs to be reorganised +*/ int mkhtemp(int *fd, struct stat *st, @@ -821,6 +832,12 @@ err: } #endif +/* TODO: potential infinite loop under entropy failure. + * e.g. keeps returning low quality RNG, or atacker + * has control (DoS attack potential). + * possible solution: add a timeout (and abort if + * the timeout is reached) + */ int mkhtemp_fill_random(char *p, size_t xc) { @@ -900,6 +917,8 @@ int secure_file(int *fd, if (lock_file(*fd, flags) == -1) goto err_demons; + /* TODO: why would this be NULL? audit + * to find out. we should always verify! */ if (expected != NULL) if (fd_verify_identity(*fd, expected, &st_now) < 0) goto err_demons; |