cleanup WIP

Signed-off-by: Leah Rowe <leah@libreboot.org>
This commit is contained in:
Leah Rowe
2026-03-22 21:08:18 +00:00
parent cc7b0a4381
commit ca92e7831a
4 changed files with 98 additions and 134 deletions

View File

@@ -478,8 +478,7 @@ const char *getnvmprogname(void);
/* libc hardening
*/
char *new_tmpfile(int *fd);
char *new_tmplate(int *fd);
int new_tmpfile(int *fd, char **path);
static int mkhtemp_try_create(int dirfd,
struct stat *st_dir_initial,
char *fname_copy,
@@ -497,9 +496,13 @@ int same_dir(const char *a, const char *b);
int tmpdir_policy(const char *path,
int *allow_noworld_unsticky);
char *env_tmpdir(int always_sticky);
int secure_file(int *fd, struct stat *st,
int bad_flags, int check_seek,
int do_lock, mode_t mode);
int secure_file(int *fd,
struct stat *st,
struct stat *expected,
int bad_flags,
int check_seek,
int do_lock,
mode_t mode);
int close_on_eintr(int fd);
int fsync_on_eintr(int fd);
int fs_rename_at(int olddirfd, const char *old,

View File

@@ -30,6 +30,9 @@
/* check that a file changed
*/
#define MKHTEMP_RETRY_MAX 512
#define MKHTEMP_SPIN_THRESHOLD 32
int
same_file(int fd, struct stat *st_old,
int check_size)
@@ -234,8 +237,10 @@ err_fsync_dir:
/* hardened tmpfile creation
*/
char *
new_tmpfile(int *fd)
/* returns opened fd, sets fd and *path at pointers *fd and *path */
/* also sets external stat */
int
new_tmpfile(int *fd, char **path)
{
/* TODO:
* directory support (currently only files)
@@ -253,7 +258,6 @@ new_tmpfile(int *fd)
char *tmpdir = NULL;
size_t dirlen;
size_t pathlen;
size_t destlen;
size_t baselen;
char *dest = NULL; /* final path */
@@ -261,43 +265,26 @@ new_tmpfile(int *fd)
int dirfd = -1;
const char *fname = NULL;
/* open the directory early,
* check via fd throughout,
* for directory replacement
* attack mitigation
*/
struct stat st_dir_initial;
char *dir = NULL;
char *base = NULL;
if (fd == NULL) {
if (path == NULL || fd == NULL) {
errno = EFAULT;
goto err_new_tmpfile;
} else if (*path == NULL) {
errno = EFAULT;
goto err_new_tmpfile;
}
/* block operating on
* someone elses file
*/
if (*fd >= 0) {
if (*fd >= 0) { /* file already opend */
errno = EEXIST;
goto err_new_tmpfile;
/* they might
* want it later!
*/
}
/* but now it's *mine*:
*/
*fd = -1;
/* base dir e.g. /tmp
*/
#if defined(PERMIT_NON_STICKY_ALWAYS) && \
((PERMIT_NON_STICKY_ALWAYS) > 0)
tmpdir = env_tmpdir(PERMIT_NON_STICKY_ALWAYS);
#else
tmpdir = env_tmpdir(0);
@@ -305,22 +292,13 @@ new_tmpfile(int *fd)
if (tmpdir == NULL)
goto err_new_tmpfile;
if (slen(tmpdir, maxlen, &dirlen) < 0)
goto err_new_tmpfile;
pathlen = 0;
/* now we want the base dir,
* with the file appended,
* and the XXXXXXXXXX suffix
*/
/* using sizeof (not slen) adds an extra byte,
* useful because we either want '.' or '/'
*/
destlen = dirlen + pathlen + sizeof(suffix);
destlen = dirlen + sizeof(suffix);
if (destlen > maxlen - 1) { /* -1 for NULL */
errno = EOVERFLOW;
@@ -334,34 +312,22 @@ new_tmpfile(int *fd)
goto err_new_tmpfile;
}
/* As you see above, we only allow
* either a base tmpdir and suffix,
* or a user-supplied file and we
* suffix that.
*/
if (dirlen > 0 && pathlen > 0) {
errno = EINVAL;
goto err_new_tmpfile; /* pre-emptive fix */
}
*(dest + dirlen) = '/';
memcpy(dest, tmpdir, dirlen);
memcpy(dest + dirlen + 1, suffix,
sizeof(suffix) - 1);
*(dest + dirlen) = '/';
memcpy(dest + dirlen + 1, suffix, sizeof(suffix) - 1);
*(dest + destlen) = '\0';
/* new tmpfile name */
fname = dest + dirlen + 1;
dirfd = fs_open(tmpdir,
O_RDONLY | O_DIRECTORY);
if (dirfd < 0)
goto err_new_tmpfile;
fname = dest + dirlen + 1;
if (fstat(dirfd, &st_dir_initial) < 0)
goto err_new_tmpfile;
*(dest + destlen) = '\0';
*fd = mkhtemp(fd, &st, dest, dirfd, fname, &st_dir_initial);
if (*fd < 0)
goto err_new_tmpfile;
@@ -370,14 +336,16 @@ new_tmpfile(int *fd)
(void) close_on_eintr(dirfd);
dirfd = -1;
}
if (dir != NULL) {
free(dir);
dir = NULL;
}
errno = saved_errno;
return dest;
*path = dest;
return 0;
err_new_tmpfile:
@@ -410,7 +378,7 @@ err_new_tmpfile:
errno = saved_errno;
return NULL;
return -1;
}
/* hardened TMPDIR parsing
@@ -712,7 +680,6 @@ mkhtemp(int *fd,
size_t xc = 0;
size_t fname_len = 0;
char *template_copy = NULL;
char *fname_copy = NULL;
char *p;
@@ -726,6 +693,7 @@ mkhtemp(int *fd,
#else
size_t max_len = 4096;
#endif
int r;
if (fd == NULL ||
template == NULL ||
@@ -793,29 +761,21 @@ mkhtemp(int *fd,
return -1;
}
template_copy = malloc(len + 1);
if (template_copy == NULL) {
errno = ENOMEM;
goto err;
}
fname_copy = malloc(fname_len + 1);
if (fname_copy == NULL) {
errno = ENOMEM;
goto err;
}
memcpy(template_copy, template, len + 1);
/* fname_copy = suffix region only; p points to trailing XXXXXX */
memcpy(fname_copy,
template_copy + len - fname_len,
template + len - fname_len,
fname_len + 1);
p = fname_copy + fname_len - xc;
for (retries = 0; retries < 300; retries++) {
for (retries = 0; retries < MKHTEMP_RETRY_MAX; retries++) {
int r = mkhtemp_try_create(
r = mkhtemp_try_create(
dirfd,
st_dir_initial,
fname_copy,
@@ -824,17 +784,16 @@ mkhtemp(int *fd,
fd,
st);
if (r == 1) {
/* success: copy final name back */
memcpy(template + len - fname_len,
fname_copy, fname_len);
/* DoS mitigation; add jitter delay (01023 µs) */
if (r != 1 && retries >= MKHTEMP_SPIN_THRESHOLD)
usleep((useconds_t)rlong() & 0x3FF);
errno = saved_errno;
goto success;
}
/* success: copy final name back */
memcpy(template + len - fname_len,
fname_copy, fname_len);
if (r == -1)
goto err;
errno = saved_errno;
goto success;
}
errno = EEXIST;
@@ -846,8 +805,6 @@ err:
}
success:
if (template_copy != NULL)
free(template_copy);
if (fname_copy != NULL)
free(fname_copy);
@@ -865,6 +822,9 @@ mkhtemp_try_create(int dirfd,
struct stat *st)
{
struct stat st_dir_now;
struct stat st_open;
int saved_errno = errno;
int close_errno;
if (mkhtemp_fill_random(p, xc) < 0)
return -1;
@@ -885,15 +845,35 @@ mkhtemp_try_create(int dirfd,
if (*fd >= 0) {
if (secure_file(fd, st, O_APPEND, 1, 1, 0600) < 0)
if (fstat(*fd, &st_open) < 0)
return -1;
if (fstat(dirfd, &st_dir_now) < 0)
return -1;
if (st_dir_now.st_dev != st_dir_initial->st_dev ||
st_dir_now.st_ino != st_dir_initial->st_ino) {
close_errno = errno;
(void) close_on_eintr(*fd);
errno = close_errno;
errno = ESTALE;
return -1;
}
if (secure_file(fd, st, &st_open, O_APPEND, 1, 1, 0600) < 0)
return -1;
errno = saved_errno;
return 1; /* success */
}
if (errno == EEXIST ||
errno == EINTR ||
errno == EAGAIN)
errno == EAGAIN ||
errno == EWOULDBLOCK ||
errno == ETXTBSY)
return 0; /* retry */
return -1; /* fatal */
@@ -964,31 +944,16 @@ err_mkhtemp_fill_random:
return -1;
}
/* why doesn't literally
every libc have this?
TODO: consider set_flags,
complementing bad_flags,
for setting new flags;
with another option to
set it before the bad_flags
check, or after it (because
some callers may be setting
flags given to them with
theirs OR'd in, yet still
want to filter bad flags,
whereas others may not want
to modify flags on a file
that already contains
specific flags)
*/
int
secure_file(int *fd,
struct stat *st, int bad_flags,
int check_seek, int do_lock,
int secure_file(int *fd,
struct stat *st,
struct stat *expected,
int bad_flags,
int check_seek,
int do_lock,
mode_t mode)
{
int flags;
struct stat st_now;
int saved_errno = errno;
if (fd == NULL) {
@@ -1012,16 +977,8 @@ secure_file(int *fd,
if (bad_flags > 0) {
/*
* For example:
* O_APPEND would permit offsets
* to be ignored, which breaks
* positional read/write
*
* You might provide that as
* the mask to this function,
* before doing positional i/o
*/
/* e.g. O_APPEND breaks pwrite/pread
* by allowing offsets to be ignored */
if (flags & bad_flags) {
errno = EPERM;
goto err_secure_file;
@@ -1031,12 +988,20 @@ secure_file(int *fd,
if (fstat(*fd, st) == -1)
goto err_secure_file;
/*
* Extremely defensive
* likely pointless checks
*/
/* verify inode/device stability */
if (expected != NULL) {
if (st_now.st_dev != expected->st_dev ||
st_now.st_ino != expected->st_ino) {
errno = ESTALE;
goto err_secure_file;
}
}
/* now copy to caller */
*st = st_now;
/* check if it's a file */
if (!S_ISREG(st->st_mode)) {
errno = EBADF;
goto err_secure_file;
@@ -1044,23 +1009,17 @@ secure_file(int *fd,
if (check_seek) {
/* check if it's seekable
*/
/* check if it's seekable */
if (lseek(*fd, 0, SEEK_CUR) == (off_t)-1)
goto err_secure_file;
}
/* tmpfile re-linked, or
unlinked, while opened
*/
/* tmpfile re/un linked while open */
if (st->st_nlink != 1) {
errno = ELOOP;
goto err_secure_file;
}
/* block if we don't own the file
* (exception made for root)
*/
if (st->st_uid != geteuid() && /* someone else's file */
geteuid() != 0) { /* override for root */

View File

@@ -473,7 +473,8 @@ gbe_mv(void)
/* create replacement temp in target directory
*/
dest_tmp = new_tmpfile(&dest_fd);
if (new_tmpfile(&dest_fd, &f->fname) < 1)
goto ret_gbe_mv;
if (dest_tmp == NULL)
goto ret_gbe_mv;

View File

@@ -112,7 +112,8 @@ xstart(int argc, char *argv[])
us.argv0 = argv[0];
us.f.fname = argv[1];
us.f.tname = new_tmpfile(&us.f.tmp_fd);
if (new_tmpfile(&us.f.tmp_fd, &us.f.tname) < 0)
err_no_cleanup(errno, "xstart: cannot create tmpfile");
/* parse user command */
/* TODO: CHECK ACCESSES VIA xstatus() */