Skip to content

Commit ceb5b22

Browse files
WavyEbuilderstephensmalley
authored andcommitted
seunshare: fix the frail tmpdir cleanup
For some reason, rm is invoked via system (3) to cleanup the runtime temp directory. This really isn't all that robust, *especially* given that seunshare is supposed to be a security boundary. Instead do this using libc, the API designed to be used within C programs. Also make sure that we don't follow symbolic links; the input being deleted is untrusted, and hence a malicious symbolic link may be placed outside of the sandbox. Signed-off-by: Rahul Sandhu <[email protected]> Acked-by: Stephen Smalley <[email protected]>
1 parent ac5e012 commit ceb5b22

File tree

1 file changed

+66
-12
lines changed

1 file changed

+66
-12
lines changed

sandbox/seunshare.c

Lines changed: 66 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
*/
55

66
#define _GNU_SOURCE
7+
#include <stdbool.h>
78
#include <signal.h>
89
#include <sys/fsuid.h>
910
#include <sys/stat.h>
@@ -403,6 +404,66 @@ static int rsynccmd(const char * src, const char *dst, char **cmdbuf)
403404
return rc;
404405
}
405406

407+
/*
408+
* Recursively delete a directory.
409+
* SAFETY: This function will NOT follow symbolic links (AT_SYMLINK_NOFOLLOW).
410+
* As a result, this function can be run safely on a directory owned by
411+
* a non-root user: symbolic links to root paths (such as /root) will
412+
* not be followed.
413+
*/
414+
static bool rm_rf(int targetfd, const char *path) {
415+
struct stat statbuf;
416+
417+
if (fstatat(targetfd, path, &statbuf, AT_SYMLINK_NOFOLLOW) < 0) {
418+
if (errno == ENOENT) {
419+
return true;
420+
}
421+
perror("fstatat");
422+
return false;
423+
}
424+
425+
if (S_ISDIR(statbuf.st_mode)) {
426+
const int newfd = openat(targetfd, path, O_RDONLY | O_DIRECTORY | O_CLOEXEC);
427+
if (newfd < 0) {
428+
perror("openat");
429+
return false;
430+
}
431+
432+
DIR *dir = fdopendir(newfd);
433+
if (!dir) {
434+
perror("fdopendir");
435+
close(newfd);
436+
return false;
437+
}
438+
439+
struct dirent *entry;
440+
int rc = true;
441+
while ((entry = readdir(dir)) != NULL) {
442+
if (strcmp(entry->d_name, ".") == 0 || strcmp(entry->d_name, "..") == 0) {
443+
continue;
444+
}
445+
446+
if (!rm_rf(dirfd(dir), entry->d_name)) {
447+
rc = false;
448+
}
449+
}
450+
451+
closedir(dir);
452+
453+
if (unlinkat(targetfd, path, AT_REMOVEDIR) < 0) {
454+
perror("unlinkat");
455+
rc = false;
456+
}
457+
458+
return rc;
459+
}
460+
if (unlinkat(targetfd, path, 0) < 0) {
461+
perror("unlinkat");
462+
return false;
463+
}
464+
return true;
465+
}
466+
406467
/**
407468
* Clean up runtime temporary directory. Returns 0 if no problem was detected,
408469
* >0 if some error was detected, but errors here are treated as non-fatal and
@@ -428,24 +489,17 @@ static int cleanup_tmpdir(const char *tmpdir, const char *src,
428489
free(cmdbuf); cmdbuf = NULL;
429490
}
430491

431-
/* remove files from the runtime temporary directory */
432-
if (asprintf(&cmdbuf, "/bin/rm -r '%s/' 2>/dev/null", tmpdir) == -1) {
433-
fprintf(stderr, _("Out of memory\n"));
434-
cmdbuf = NULL;
492+
if ((uid_t)setfsuid(0) != 0) {
493+
/* setfsuid does not return error, but this check makes code checkers happy */
435494
rc++;
436495
}
437-
/* this may fail if there's root-owned file left in the runtime tmpdir */
438-
if (cmdbuf && spawn_command(cmdbuf, pwd->pw_uid) != 0) rc++;
439-
free(cmdbuf); cmdbuf = NULL;
440496

441-
/* remove runtime temporary directory */
442-
if ((uid_t)setfsuid(0) != 0) {
443-
/* setfsuid does not return error, but this check makes code checkers happy */
497+
/* Recursively remove the runtime temp directory. */
498+
if (!rm_rf(AT_FDCWD, tmpdir)) {
499+
fprintf(stderr, _("Failed to recursively remove directory %s\n"), tmpdir);
444500
rc++;
445501
}
446502

447-
if (pwd->pw_uid != 0 && rmdir(tmpdir) == -1)
448-
fprintf(stderr, _("Failed to remove directory %s: %s\n"), tmpdir, strerror(errno));
449503
if ((uid_t)setfsuid(pwd->pw_uid) != 0) {
450504
fprintf(stderr, _("unable to switch back to user after clearing tmp dir\n"));
451505
rc++;

0 commit comments

Comments
 (0)