From b41532ab6c8578fd6df254a41d1be54c9331aa3f Mon Sep 17 00:00:00 2001 From: smitsohu Date: Mon, 3 Aug 2020 19:20:32 +0200 Subject: don't run with closed standard streams Ensure that all standard streams are open and we don't inadvertently print to files opened for a different reason; in general we can expect glibc to take care of this, but it doesn't cover the case where a sandbox is started by root. The added code also serves as a fallback. Unrelated: For what it's worth, shift umask call closer to main start, so it runs before lowering privileges and before anything can really go wrong. --- src/firejail/main.c | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/src/firejail/main.c b/src/firejail/main.c index 958374c43..79e39b669 100644 --- a/src/firejail/main.c +++ b/src/firejail/main.c @@ -361,6 +361,24 @@ static void init_cfg(int argc, char **argv) { cfg.seccomp_error_action = "EPERM"; } +static void fix_single_std_fd(int fd, const char *file, int flags) { + struct stat s; + if (fstat(fd, &s) == -1 && errno == EBADF) { + // something is wrong with fd, probably it is not opened + int nfd = open(file, flags); + if (nfd != fd || fstat(fd, &s) != 0) + _exit(1); // no further attempts to fix the situation + } +} + +// glibc does this automatically if Firejail was started by a regular user +// run this for root user and as a fallback +static void fix_std_streams(void) { + fix_single_std_fd(0, "/dev/full", O_RDONLY|O_NOFOLLOW); + fix_single_std_fd(1, "/dev/null", O_WRONLY|O_NOFOLLOW); + fix_single_std_fd(2, "/dev/null", O_WRONLY|O_NOFOLLOW); +} + static void check_network(Bridge *br) { assert(br); if (br->macvlan == 0) // for bridge devices check network range or arp-scan and assign address @@ -1017,16 +1035,19 @@ int main(int argc, char **argv, char **envp) { int arg_caps_cmdline = 0; // caps requested on command line (used to break out of --chroot) char **ptr; + // sanitize the umask + orig_umask = umask(022); + + // check standard streams before printing anything + fix_std_streams(); + // drop permissions by default and rise them when required EUID_INIT(); EUID_USER(); - // sanitize the umask - orig_umask = umask(022); - // argument count should be larger than 0 if (argc == 0 || !argv || strlen(argv[0]) == 0) { - fprintf(stderr, "Error: argv[0] is NULL\n"); + fprintf(stderr, "Error: argv is invalid\n"); exit(1); } else if (argc >= MAX_ARGS) { fprintf(stderr, "Error: too many arguments\n"); -- cgit v1.2.3-70-g09d2