diff --git a/src/firejail/fs.c b/src/firejail/fs.c index 31776bc1a..bc5bb2e88 100644 --- a/src/firejail/fs.c +++ b/src/firejail/fs.c @@ -60,11 +60,11 @@ static int disable_file(OPERATION op, const char *filename) { EUID_ASSERT(); // Resolve all symlinks - char* fname = realpath(filename, NULL); - if (fname == NULL && errno != EACCES) { + char* rpath = realpath(filename, NULL); + if (rpath == NULL && errno != EACCES) { return 1; } - if (fname == NULL && errno == EACCES) { + if (rpath == NULL && errno == EACCES) { // realpath and stat functions will fail on FUSE filesystems // they don't seem to like a uid of 0 // force mounting @@ -95,22 +95,38 @@ static int disable_file(OPERATION op, const char *filename) { } } - assert(fname); + assert(rpath); // check for firejail executable // we might have a file found in ${PATH} pointing to /usr/bin/firejail // blacklisting it here will end up breaking situations like user clicks on a link in Thunderbird // and expects Firefox to open in the same sandbox - if (strcmp(BINDIR "/firejail", fname) == 0) { - free(fname); + if (strcmp(BINDIR "/firejail", rpath) == 0) { + free(rpath); return 1; } + int follow_symlinks = 1; + + // Users may try to disable writing to certain paths in the user home + // by replacing them with symlinks to /dev/null, so avoid following + // symlinks to /dev/null to prevent disabling /dev/null itself, as that + // can lead to unexpected issues (see #5803). + if (strcmp(rpath, "/dev/null") == 0 && is_link(filename)) + follow_symlinks = 0; + + int extraflags = 0; + const char *fname = rpath; // filename to be used for operations + if (!follow_symlinks) { + fname = filename; + extraflags |= O_NOFOLLOW; + } + // if the file is not present, do nothing - int fd = open(fname, O_PATH|O_CLOEXEC); + int fd = open(fname, O_PATH|O_CLOEXEC|extraflags); if (fd < 0) { if (arg_debug) printf("Warning (blacklisting): cannot open %s: %s\n", fname, strerror(errno)); - free(fname); + free(rpath); return 1; } @@ -118,7 +134,7 @@ static int disable_file(OPERATION op, const char *filename) { if (fstat(fd, &s) < 0) { if (arg_debug) printf("Warning (blacklisting): cannot stat %s: %s\n", fname, strerror(errno)); - free(fname); + free(rpath); close(fd); return 1; } @@ -214,7 +230,7 @@ static int disable_file(OPERATION op, const char *filename) { out: close(fd); - free(fname); + free(rpath); return retval; } diff --git a/src/man/firejail-profile.5.in b/src/man/firejail-profile.5.in index 35b8d5d2b..0981809de 100644 --- a/src/man/firejail-profile.5.in +++ b/src/man/firejail-profile.5.in @@ -266,6 +266,13 @@ Symbolic link handling: Blacklisting a path that is a symbolic link will also blacklist the path that it points to. For example, if ~/foo is blacklisted and it points to /bar, then /bar will also be blacklisted. +Symlinks to /dev/null are not followed (only the symlink itself will be +affected), as replacing certain paths with symlinks to /dev/null is not +uncommon and restricting access to /dev/null can lead to unexpected issues (see +.UR https://github.com/netblue30/firejail/issues/5803 +#5803 +.UE ). +Using /dev/null directly (instead of through a symlink) should work normally. .br .br diff --git a/src/man/firejail.1.in b/src/man/firejail.1.in index 6f61209ee..83ba5de7b 100644 --- a/src/man/firejail.1.in +++ b/src/man/firejail.1.in @@ -247,6 +247,13 @@ Symbolic link handling: Blacklisting a path that is a symbolic link will also blacklist the path that it points to. For example, if ~/foo is blacklisted and it points to /bar, then /bar will also be blacklisted. +Symlinks to /dev/null are not followed (only the symlink itself will be +affected), as replacing certain paths with symlinks to /dev/null is not +uncommon and restricting access to /dev/null can lead to unexpected issues (see +.UR https://github.com/netblue30/firejail/issues/5803 +#5803 +.UE ). +Using /dev/null directly (instead of through a symlink) should work normally. .br .br diff --git a/test/fs/devnull_symlink b/test/fs/devnull_symlink new file mode 120000 index 000000000..dc1dc0cde --- /dev/null +++ b/test/fs/devnull_symlink @@ -0,0 +1 @@ +/dev/null \ No newline at end of file diff --git a/test/fs/disable-devnull-symlink.exp b/test/fs/disable-devnull-symlink.exp new file mode 100755 index 000000000..577e9d2e5 --- /dev/null +++ b/test/fs/disable-devnull-symlink.exp @@ -0,0 +1,57 @@ +#!/usr/bin/expect -f +# This file is part of Firejail project +# Copyright (C) 2014-2026 Firejail Authors +# License GPL v2 + +set timeout 2 +spawn $env(SHELL) +match_max 100000 +set PWD $env(PWD) + +send -- "ls -l /dev/null\r" +expect { + timeout {puts "TESTING ERROR 0\n";exit} + "crw-rw-rw-" {puts "OK\n"} +} +after 100 + +# Test that disabling a symlink to /dev/null does not disable /dev/null itself +# (see #5803). +send -- "firejail --noprofile --blacklist=./devnull_symlink ls -l /dev/null\r" +expect { + timeout {puts "TESTING ERROR 1\n";exit} + "crw-rw-rw-" {puts "OK\n"} + "cr--r--r--" {puts "TESTING ERROR 2\n";exit} +} +after 100 + +send -- "firejail --noprofile --blacklist=./devnull_symlink touch /dev/null; echo ret \$?\r" +expect { + timeout {puts "TESTING ERROR 3\n";exit} + -re {ret 0} {puts "OK\n"} + -re {ret [1-9]} {puts "TESTING ERROR 4\n";exit} +} +after 100 + +# Test that /dev/null can still be disabled directly (without a symlink). +send -- "firejail --noprofile --blacklist=/dev/null ls -l /dev/null\r" +expect { + timeout {puts "TESTING ERROR 5\n";exit} + "\\-r--------" {puts "OK\n"} + "crw-rw-rw-" {puts "TESTING ERROR 6\n";exit} +} +after 100 + +send -- "firejail --noprofile --blacklist=/dev/null touch /dev/null; echo ret \$?\r" +expect { + timeout {puts "TESTING ERROR 7\n";exit} + -re {ret 0} {puts "TESTING ERROR 8\n";exit} + -re {ret [1-9]} {puts "OK\n"} +} +after 100 + +send -- "exit\r" + +after 100 + +puts "\nall done\n" diff --git a/test/fs/fs.sh b/test/fs/fs.sh index 9d3f6e196..02df5fbb1 100755 --- a/test/fs/fs.sh +++ b/test/fs/fs.sh @@ -125,6 +125,9 @@ echo "TESTING: blacklist glob (test/fs/option_blacklist_glob.exp)" ./option_blacklist_glob.exp rm -fr ~/_firejail_test_dir +echo "TESTING: disable symlink to /dev/null (test/fs/disable-devnull-symlink.exp)" +./disable-devnull-symlink.exp + echo "TESTING: noblacklist blacklist noexec (test/fs/noblacklist-blacklist-noexec.exp)" ./noblacklist-blacklist-noexec.exp