From 9d41a5745242e16d3a3b71269e547147321b5570 Mon Sep 17 00:00:00 2001 From: Vincent550102 <5020559@gmail.com> Date: Mon, 11 May 2026 20:04:39 +0800 Subject: [PATCH] Reject directory stdio fds unless explicitly passed --- Makefile | 2 ++ cmdline.cc | 9 ++++++--- config.cc | 1 + contain.cc | 30 ++++++++++++++++++++++++++++++ nsjail.h | 1 + 5 files changed, 40 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index da68cc55..8375de7c 100644 --- a/Makefile +++ b/Makefile @@ -152,6 +152,8 @@ test: $(BIN) # --- Basic sanity tests --- $(call run_test, ./nsjail -q -Mo --chroot / --user 99999 --group 99999 -- /bin/true, 0) $(call run_test, ./nsjail -q -Mo --chroot / --user 99999 --group 99999 -- /bin/false, 1) + $(call run_test, ./nsjail -q -Mo --chroot / --user 99999 --group 99999 -- /bin/true < /tmp, 255) + $(call run_test, ./nsjail -q -Mo --chroot / --user 99999 --group 99999 --pass_fd 0 -- /bin/true < /tmp, 0) $(call run_test, ./nsjail --config tests/seccomp.cfg -q -t 2 -- /bin/bash -c 'strace -o /dev/null /bin/true || exit 77', 77) $(call run_test, ./nsjail --config tests/basic.cfg -q -t 2 -- /bin/bash -c 'strace -o /dev/null /bin/true && exit 77', 77) $(call run_test, ./nsjail --config tests/pasta-nat.cfg -q -t 3 -- /bin/bash -c 'sleep 0.2; ping -W 1 -c 1 8.8.8.8 && exit 77', 77) diff --git a/cmdline.cc b/cmdline.cc index 637b03e1..b944fc51 100644 --- a/cmdline.cc +++ b/cmdline.cc @@ -104,7 +104,7 @@ static const struct custom_option custom_opts[] = { { { "silent", no_argument, nullptr, 0x0502 }, "Redirect child process' fd:0/1/2 to /dev/null" }, { { "stderr_to_null", no_argument, nullptr, 0x0503 }, "Redirect child process' fd:2 (STDERR_FILENO) to /dev/null" }, { { "skip_setsid", no_argument, nullptr, 0x0504 }, "Don't call setsid(), allows for terminal signal handling in the sandboxed process. Dangerous" }, - { { "pass_fd", required_argument, nullptr, 0x0505 }, "Don't close this FD before executing the child process (can be specified multiple times), by default: 0/1/2 are kept open" }, + { { "pass_fd", required_argument, nullptr, 0x0505 }, "Don't close this FD before executing the child process (can be specified multiple times), by default: 0/1/2 are kept open, but directory stdio fds are rejected unless explicitly passed" }, { { "disable_no_new_privs", no_argument, nullptr, 0x0507 }, "Don't set the prctl(NO_NEW_PRIVS, 1) (DANGEROUS)" }, { { "rlimit_as", required_argument, nullptr, 0x0201 }, "RLIMIT_AS in MB, 'max' or 'hard' for the current hard limit, 'def' or 'soft' for the current soft limit, 'inf' for RLIM64_INFINITY (default: 4096)" }, { { "rlimit_core", required_argument, nullptr, 0x0202 }, "RLIMIT_CORE in MB, 'max' or 'hard' for the current hard limit, 'def' or 'soft' for the current soft limit, 'inf' for RLIM64_INFINITY (default: 0)" }, @@ -687,8 +687,11 @@ std::unique_ptr parseArgs(int argc, char* argv[]) { case 0x0504: nsj->njc.set_skip_setsid(true); break; - case 0x0505: - nsj->openfds.push_back((int)strtol(optarg, NULL, 0)); + case 0x0505: { + int fd = (int)strtol(optarg, NULL, 0); + nsj->openfds.push_back(fd); + nsj->passfds.push_back(fd); + } break; case 0x0507: nsj->njc.set_disable_no_new_privs(true); diff --git a/config.cc b/config.cc index 2d28584e..aaaf0a17 100644 --- a/config.cc +++ b/config.cc @@ -103,6 +103,7 @@ static bool parseInternal(nsj_t* nsj, const nsjail::NsJailConfig& njc) { } for (ssize_t i = 0; i < njc.pass_fd_size(); i++) { nsj->openfds.push_back(njc.pass_fd(i)); + nsj->passfds.push_back(njc.pass_fd(i)); } for (ssize_t i = 0; i < njc.uidmap_size(); i++) { if (!user::parseId(nsj, njc.uidmap(i).inside_id(), njc.uidmap(i).outside_id(), diff --git a/contain.cc b/contain.cc index a3245de9..32c0140d 100644 --- a/contain.cc +++ b/contain.cc @@ -35,6 +35,7 @@ #include #include #include +#include #include #include @@ -225,6 +226,34 @@ static bool containPassFd(nsj_t* nsj, int fd) { return (std::find(nsj->openfds.begin(), nsj->openfds.end(), fd) != nsj->openfds.end()); } +static bool containExplicitPassFd(nsj_t* nsj, int fd) { + return (std::find(nsj->passfds.begin(), nsj->passfds.end(), fd) != nsj->passfds.end()); +} + +static bool containValidateDefaultStdioFds(nsj_t* nsj) { + for (int fd = STDIN_FILENO; fd <= STDERR_FILENO; fd++) { + if (containExplicitPassFd(nsj, fd)) { + continue; + } + + struct stat st; + if (TEMP_FAILURE_RETRY(fstat(fd, &st)) == -1) { + if (errno == EBADF) { + continue; + } + PLOG_E("fstat(fd=%d)", fd); + return false; + } + if (S_ISDIR(st.st_mode)) { + LOG_E("Default stdio fd=%d is a directory. Refusing to pass a directory fd " + "into the sandbox; use --pass_fd=%d to pass it explicitly", + fd, fd); + return false; + } + } + return true; +} + static bool containMakeFdCOE(int fd, bool pass_fd) { int flags = TEMP_FAILURE_RETRY(fcntl(fd, F_GETFD, 0)); if (flags == -1) { @@ -321,6 +350,7 @@ static bool containMakeFdsCOEProc(nsj_t* nsj) { } static bool containMakeFdsCOE(nsj_t* nsj) { + RETURN_ON_FAILURE(containValidateDefaultStdioFds(nsj)); if (containMakeFdsCOECloseRange(nsj)) { return true; } diff --git a/nsjail.h b/nsjail.h index a17ac287..2aa4cbec 100644 --- a/nsjail.h +++ b/nsjail.h @@ -95,6 +95,7 @@ struct nsj_t { std::vector uids; std::vector gids; std::vector openfds; + std::vector passfds; std::vector pipes; int exit_status;