mirror of
https://github.com/google/nsjail.git
synced 2026-08-30 18:41:30 -07:00
Reject path traversal in mount destinations during jail staging
Mount destinations were joined as newroot+"/"+dst and walked with mkdirat/openat without rejecting "."/".." components or following symlinks safely. An attacker who can influence mount dst (CLI, config, or prefix_dst_env) could create directories, symlinks, or mounts outside the intended staging root on the host before pivot_root. Add isSafeContainmentPath(), validate destinations in both legacy and new mount APIs, use O_NOFOLLOW while walking parents, and include a standalone regression test.
This commit is contained in:
@@ -82,6 +82,13 @@ static mount_t prepareMountPoint(const nsjail::MountPt& proto) {
|
||||
}
|
||||
mpt.dst += proto.dst();
|
||||
|
||||
if (!util::isSafeContainmentPath(mpt.dst)) {
|
||||
LOG_E("Mount destination escapes containment via '.'/'..'/NUL: %s", QC(mpt.dst));
|
||||
/* Keep the unsafe dst so later mount steps fail closed rather than
|
||||
* treating a cleared path as the jail root. */
|
||||
return mpt;
|
||||
}
|
||||
|
||||
mpt.flags = proto.rw() ? 0 : (uintptr_t)MS_RDONLY;
|
||||
if (proto.is_bind()) {
|
||||
mpt.flags |= MS_BIND | MS_REC | MS_PRIVATE;
|
||||
@@ -196,7 +203,16 @@ static bool mountWithDynamicContent(
|
||||
static bool mountSinglePoint(mount_t* mpt, const char* newroot, const char* tmpdir) {
|
||||
LOG_D("Mounting (legacy): %s", mnt::describeMountPt(*mpt->mpt).c_str());
|
||||
|
||||
if (!util::isSafeContainmentPath(mpt->dst)) {
|
||||
LOG_E("Mount destination escapes containment via '.'/'..'/NUL: %s", QC(mpt->dst));
|
||||
return false;
|
||||
}
|
||||
|
||||
const std::string dstpath = std::string(newroot) + "/" + mpt->dst;
|
||||
if (!util::isSafeContainmentPath(dstpath)) {
|
||||
LOG_E("Resolved mount path escapes containment: %s", QC(dstpath));
|
||||
return false;
|
||||
}
|
||||
std::string srcpath = mpt->src.empty() ? "none" : mpt->src;
|
||||
|
||||
if (!util::createDirRecursively(dstpath.c_str())) {
|
||||
|
||||
@@ -199,6 +199,10 @@ static bool createDirAt(int dir_fd, const char* path, mode_t mode) {
|
||||
if (!path[0]) {
|
||||
return true;
|
||||
}
|
||||
if (!util::isSafeContainmentPath(path)) {
|
||||
LOG_E("Mount destination escapes containment via '.'/'..'/NUL: '%s'", path);
|
||||
return false;
|
||||
}
|
||||
|
||||
std::string cumulative;
|
||||
for (const auto& component : util::strSplit(path, '/')) {
|
||||
@@ -412,8 +416,12 @@ static bool mountSinglePointAt(mount_t* mpt, int root_fd) {
|
||||
LOG_D("Mounting (new API): %s", mnt::describeMountPt(*mpt->mpt).c_str());
|
||||
|
||||
const char* rel_dst = util::stripLeadingSlashes(mpt->dst.c_str());
|
||||
/* Empty / only-slashes dst means the jail root (represented as "."). */
|
||||
if (!rel_dst[0]) {
|
||||
rel_dst = ".";
|
||||
} else if (!util::isSafeContainmentPath(rel_dst)) {
|
||||
LOG_E("Mount destination escapes containment via '.'/'..'/NUL: %s", QC(mpt->dst));
|
||||
return false;
|
||||
}
|
||||
|
||||
const char* last_slash = strrchr(rel_dst, '/');
|
||||
@@ -504,6 +512,13 @@ static mount_t prepareMountPoint(const nsjail::MountPt& proto) {
|
||||
}
|
||||
mpt.dst += proto.dst();
|
||||
|
||||
if (!util::isSafeContainmentPath(mpt.dst)) {
|
||||
LOG_E("Mount destination escapes containment via '.'/'..'/NUL: %s", QC(mpt.dst));
|
||||
/* Keep the unsafe dst so later mount steps fail closed rather than
|
||||
* treating a cleared path as the jail root. */
|
||||
return mpt;
|
||||
}
|
||||
|
||||
mpt.flags = proto.rw() ? 0 : (uintptr_t)MS_RDONLY;
|
||||
if (proto.is_bind()) {
|
||||
mpt.flags |= MS_BIND | MS_REC | MS_PRIVATE;
|
||||
|
||||
156
tests/path_containment_test.cc
Normal file
156
tests/path_containment_test.cc
Normal file
@@ -0,0 +1,156 @@
|
||||
/*
|
||||
* Regression tests for mount-destination path traversal hardening.
|
||||
*
|
||||
* Standalone harness (mirrors util::isSafeContainmentPath /
|
||||
* createDirRecursively policy) so it can run without linking full nsjail.
|
||||
*
|
||||
* g++ -std=c++20 -O1 -o path_containment_test tests/path_containment_test.cc
|
||||
* ./path_containment_test
|
||||
*/
|
||||
|
||||
#include <cstdio>
|
||||
#include <cstring>
|
||||
#include <fcntl.h>
|
||||
#include <string>
|
||||
#include <sys/stat.h>
|
||||
#include <unistd.h>
|
||||
#include <vector>
|
||||
|
||||
namespace {
|
||||
|
||||
std::vector<std::string> strSplit(const std::string& str, char delim) {
|
||||
std::vector<std::string> vec;
|
||||
std::string word;
|
||||
for (char c : str) {
|
||||
if (c == delim) {
|
||||
vec.push_back(word);
|
||||
word.clear();
|
||||
} else {
|
||||
word.push_back(c);
|
||||
}
|
||||
}
|
||||
vec.push_back(word);
|
||||
return vec;
|
||||
}
|
||||
|
||||
bool isSafeContainmentPath(const std::string& path) {
|
||||
if (path.empty()) {
|
||||
return true;
|
||||
}
|
||||
if (path.find('\0') != std::string::npos) {
|
||||
return false;
|
||||
}
|
||||
for (const auto& component : strSplit(path, '/')) {
|
||||
if (component.empty()) {
|
||||
continue;
|
||||
}
|
||||
if (component == "." || component == "..") {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
bool createDirRecursivelySafe(const char* dir) {
|
||||
if (dir[0] != '/') {
|
||||
return false;
|
||||
}
|
||||
if (!isSafeContainmentPath(dir)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
int prev_dir_fd = open("/", O_RDONLY | O_CLOEXEC | O_DIRECTORY);
|
||||
if (prev_dir_fd == -1) {
|
||||
return false;
|
||||
}
|
||||
|
||||
char path[4096];
|
||||
if (snprintf(path, sizeof(path), "%s", dir) >= (int)sizeof(path)) {
|
||||
close(prev_dir_fd);
|
||||
return false;
|
||||
}
|
||||
char* curr = path;
|
||||
for (;;) {
|
||||
while (*curr == '/') {
|
||||
curr++;
|
||||
}
|
||||
char* next = strchr(curr, '/');
|
||||
if (next == nullptr) {
|
||||
close(prev_dir_fd);
|
||||
return true;
|
||||
}
|
||||
*next = '\0';
|
||||
if (mkdirat(prev_dir_fd, curr, 0755) == -1 && errno != EEXIST) {
|
||||
close(prev_dir_fd);
|
||||
return false;
|
||||
}
|
||||
int dir_fd = openat(prev_dir_fd, curr, O_DIRECTORY | O_CLOEXEC | O_NOFOLLOW);
|
||||
if (dir_fd == -1) {
|
||||
close(prev_dir_fd);
|
||||
return false;
|
||||
}
|
||||
close(prev_dir_fd);
|
||||
prev_dir_fd = dir_fd;
|
||||
curr = next + 1;
|
||||
}
|
||||
}
|
||||
|
||||
} // namespace
|
||||
|
||||
static void expect_safe(const char* p, bool want) {
|
||||
bool got = isSafeContainmentPath(p);
|
||||
if (got != want) {
|
||||
fprintf(stderr, "FAIL isSafeContainmentPath(%s) = %d want %d\n", p, got, want);
|
||||
exit(1);
|
||||
}
|
||||
}
|
||||
|
||||
int main() {
|
||||
expect_safe("", true);
|
||||
expect_safe("/", true);
|
||||
expect_safe("/usr/lib", true);
|
||||
expect_safe("/tmp/nsjail.root/home/user", true);
|
||||
expect_safe("usr/lib", true);
|
||||
expect_safe("/usr//lib", true);
|
||||
|
||||
expect_safe("..", false);
|
||||
expect_safe("/..", false);
|
||||
expect_safe("/../", false);
|
||||
expect_safe("/tmp/../etc", false);
|
||||
expect_safe("/tmp/nsjail.root/../../etc/passwd", false);
|
||||
expect_safe("/foo/./bar", false);
|
||||
expect_safe("./foo", false);
|
||||
expect_safe("foo/../../bar", false);
|
||||
|
||||
std::string with_nul = std::string("/tmp/foo") + '\0' + "bar";
|
||||
if (isSafeContainmentPath(with_nul)) {
|
||||
fprintf(stderr, "FAIL NUL component accepted\n");
|
||||
return 1;
|
||||
}
|
||||
|
||||
(void)system("rm -rf /tmp/nsj_path_test /tmp/nsj_path_escaped");
|
||||
mkdir("/tmp/nsj_path_test", 0755);
|
||||
const char* escape =
|
||||
"/tmp/nsj_path_test/root/../../nsj_path_escaped/evil_dir/leaf";
|
||||
if (createDirRecursivelySafe(escape)) {
|
||||
fprintf(stderr, "FAIL createDirRecursively accepted traversal path\n");
|
||||
return 1;
|
||||
}
|
||||
if (access("/tmp/nsj_path_escaped", F_OK) == 0) {
|
||||
fprintf(stderr, "FAIL escape directory was created\n");
|
||||
return 1;
|
||||
}
|
||||
|
||||
const char* ok = "/tmp/nsj_path_test/root/home/user/docs/leaf";
|
||||
if (!createDirRecursivelySafe(ok)) {
|
||||
fprintf(stderr, "FAIL createDirRecursively rejected safe path\n");
|
||||
return 1;
|
||||
}
|
||||
if (access("/tmp/nsj_path_test/root/home/user/docs", F_OK) != 0) {
|
||||
fprintf(stderr, "FAIL safe parents were not created\n");
|
||||
return 1;
|
||||
}
|
||||
|
||||
printf("OK path_containment_test passed\n");
|
||||
return 0;
|
||||
}
|
||||
31
util.cc
31
util.cc
@@ -232,11 +232,35 @@ bool writeBufToFile(
|
||||
return true;
|
||||
}
|
||||
|
||||
bool isSafeContainmentPath(const std::string& path) {
|
||||
/* Empty / root-only paths refer to the containment root itself. */
|
||||
if (path.empty()) {
|
||||
return true;
|
||||
}
|
||||
/* Embedded NUL would truncate C-string APIs and hide trailing components. */
|
||||
if (path.find('\0') != std::string::npos) {
|
||||
return false;
|
||||
}
|
||||
for (const auto& component : strSplit(path, '/')) {
|
||||
if (component.empty()) {
|
||||
continue;
|
||||
}
|
||||
if (component == "." || component == "..") {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
bool createDirRecursively(const char* dir) {
|
||||
if (dir[0] != '/') {
|
||||
LOG_W("The directory path must start with '/': '%s' provided", dir);
|
||||
return false;
|
||||
}
|
||||
if (!isSafeContainmentPath(dir)) {
|
||||
LOG_W("Refusing path with '.'/'..'/NUL components: '%s'", dir);
|
||||
return false;
|
||||
}
|
||||
|
||||
int prev_dir_fd = TEMP_FAILURE_RETRY(open("/", O_RDONLY | O_CLOEXEC | O_DIRECTORY));
|
||||
if (prev_dir_fd == -1) {
|
||||
@@ -271,9 +295,12 @@ bool createDirRecursively(const char* dir) {
|
||||
}
|
||||
}
|
||||
|
||||
int dir_fd = TEMP_FAILURE_RETRY(openat(prev_dir_fd, curr, O_DIRECTORY | O_CLOEXEC));
|
||||
/* O_NOFOLLOW: do not walk through symlinks that escape the intended tree. */
|
||||
int dir_fd = TEMP_FAILURE_RETRY(
|
||||
openat(prev_dir_fd, curr, O_DIRECTORY | O_CLOEXEC | O_NOFOLLOW));
|
||||
if (dir_fd == -1) {
|
||||
PLOG_W("openat('%d', %s, O_DIRECTORY | O_CLOEXEC)", prev_dir_fd, QC(curr));
|
||||
PLOG_W("openat('%d', %s, O_DIRECTORY | O_CLOEXEC | O_NOFOLLOW)", prev_dir_fd,
|
||||
QC(curr));
|
||||
close(prev_dir_fd);
|
||||
return false;
|
||||
}
|
||||
|
||||
3
util.h
3
util.h
@@ -60,6 +60,9 @@ int recvFd(int sock);
|
||||
bool writeBufToFile(
|
||||
const char* filename, const void* buf, size_t len, int open_flags, bool log_errors = true);
|
||||
bool createDirRecursively(const char* dir);
|
||||
/* Reject ".", ".." and embedded NUL in mount destinations so they cannot
|
||||
* escape the jail staging root via path traversal. */
|
||||
bool isSafeContainmentPath(const std::string& path);
|
||||
std::string* StrAppend(std::string* str, const char* format, ...)
|
||||
__attribute__((format(printf, 2, 3)));
|
||||
std::string StrPrintf(const char* format, ...) __attribute__((format(printf, 1, 2)));
|
||||
|
||||
Reference in New Issue
Block a user