diff options
Diffstat (limited to 'util/libreboot-utils/lib/mkhtemp.c')
| -rw-r--r-- | util/libreboot-utils/lib/mkhtemp.c | 110 |
1 files changed, 50 insertions, 60 deletions
diff --git a/util/libreboot-utils/lib/mkhtemp.c b/util/libreboot-utils/lib/mkhtemp.c index c913ce6c..61479b25 100644 --- a/util/libreboot-utils/lib/mkhtemp.c +++ b/util/libreboot-utils/lib/mkhtemp.c @@ -144,13 +144,7 @@ new_tmp_common(int *fd, char **path, int type, goto err; } - dest = malloc(destlen + 1); - if (dest == NULL) { - errno = ENOMEM; - goto err; - } - - memcpy(dest, tmpdir, dirlen); + memcpy(smalloc(&dest, destlen + 1), tmpdir, dirlen); *(dest + dirlen) = '/'; memcpy(dest + dirlen + 1, templatestr, templatestr_len); *(dest + destlen) = '\0'; @@ -170,7 +164,7 @@ new_tmp_common(int *fd, char **path, int type, if (*fd < 0) goto err; - close_no_err(&dirfd); + close_on_eintr(&dirfd); errno = saved_errno; *path = dest; @@ -186,8 +180,8 @@ err: free_and_set_null(&dest); - close_no_err(&dirfd); - close_no_err(fd); + close_on_eintr(&dirfd); + close_on_eintr(fd); /* where a TMPDIR isn't found, and we err, * we pass this back through for the @@ -212,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) @@ -244,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; @@ -301,11 +295,7 @@ tmpdir_policy(const char *path, return 0; err_tmpdir_policy: - - if (errno == saved_errno) - errno = EIO; - - return -1; + return set_errno(saved_errno, EIO); } int @@ -337,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; @@ -354,8 +344,8 @@ same_dir(const char *a, const char *b) if (st_a.st_dev == st_b.st_dev && st_a.st_ino == st_b.st_ino) { - close_no_err(&fd_a); - close_no_err(&fd_b); + close_on_eintr(&fd_a); + close_on_eintr(&fd_b); success_same_dir: @@ -366,8 +356,8 @@ success_same_dir: return 1; } - close_no_err(&fd_a); - close_no_err(&fd_b); + close_on_eintr(&fd_a); + close_on_eintr(&fd_b); /* FAILURE (logical) */ @@ -380,13 +370,10 @@ err_same_dir: /* FAILURE (probably syscall) */ - close_no_err(&fd_a); - close_no_err(&fd_b); + close_on_eintr(&fd_a); + close_on_eintr(&fd_b); - if (errno == saved_errno) - errno = EIO; - - return -1; + return set_errno(saved_errno, EIO); } /* bypass_all_sticky_checks: if set, @@ -473,20 +460,15 @@ world_writeable_and_sticky( goto sticky_hell; /* heaven visa denied */ sticky_heaven: - - close_no_err(&dirfd); + close_on_eintr(&dirfd); errno = saved_errno; return 1; sticky_hell: + close_on_eintr(&dirfd); - if (errno == saved_errno) - errno = EPERM; - - close_no_err(&dirfd); - errno = saved_errno; - + (void) set_errno(saved_errno, EPERM); return 0; } @@ -531,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, @@ -585,11 +578,8 @@ mkhtemp(int *fd, fname_len) != 0, EINVAL)) return -1; - if((fname_copy = malloc(fname_len + 1)) == NULL) - goto err; - /* fname_copy = templatestr region only; p points to trailing XXXXXX */ - memcpy(fname_copy, + memcpy(smalloc(&fname_copy, fname_len + 1), template + len - fname_len, fname_len + 1); p = fname_copy + fname_len - xc; @@ -616,7 +606,7 @@ mkhtemp(int *fd, errno = EEXIST; err: - close_no_err(fd); + close_on_eintr(fd); success: free_and_set_null(&fname_copy); @@ -742,7 +732,7 @@ mkhtemp_try_create(int dirfd, goto out; err: - close_no_err(fd); + close_on_eintr(fd); if (file_created) (void) unlinkat(dirfd, fname_copy, 0); @@ -837,11 +827,17 @@ err: if (linked) (void) unlinkat(dirfd, fname_copy, 0); - close_no_err(&tmpfd); + close_on_eintr(&tmpfd); return -1; } #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) { @@ -921,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; @@ -933,11 +931,7 @@ int secure_file(int *fd, return 0; err_demons: - - if (errno == saved_errno) - errno = EIO; - - return -1; + return set_errno(saved_errno, EIO); } int @@ -1047,9 +1041,5 @@ lock_file(int fd, int flags) return 0; err_lock_file: - - if (errno == saved_errno) - errno = EIO; - - return -1; + return set_errno(saved_errno, EIO); } |