diff --git a/src/permission/fs_permission.cc b/src/permission/fs_permission.cc index 98146cf825ad38..27f27a00024615 100644 --- a/src/permission/fs_permission.cc +++ b/src/permission/fs_permission.cc @@ -304,7 +304,11 @@ bool FSPermission::RadixTree::Lookup(const std::string_view& s, auto node = current_node->NextNode(path, parent_node_prefix_len); if (node == nullptr) { - return false; + // The whole path matched this node. Grant it when the directory itself + // was allowed through a trailing wildcard ("/dir/*"), which after a + // radix split lives on a descendant rather than on this node. + return parent_node_prefix_len == path_len && + current_node->HasWildcardGrantForSelf(); } current_node = node; diff --git a/src/permission/fs_permission.h b/src/permission/fs_permission.h index 19d72ef654c9f0..135afb3514fd72 100644 --- a/src/permission/fs_permission.h +++ b/src/permission/fs_permission.h @@ -68,7 +68,13 @@ class FSPermission final : public PermissionBase { return split_child->CreateChild(path_prefix.substr(i)); } } - child->is_leaf = true; + // Only mark the child as a leaf when it already terminates an + // inserted path. Routing a longer path through this node must not + // turn a pass-through prefix into a granted entry, or an unrelated + // parent directory would be treated as allowed. + if (child->IsEndNode()) { + child->is_leaf = true; + } return child->CreateChild(path_prefix.substr(i)); } @@ -137,6 +143,31 @@ class FSPermission final : public PermissionBase { } return is_leaf; } + + // A directory allowed with a trailing wildcard ("/dir/*") also grants + // access to the directory itself. After a radix split that grant is + // stored as a "/" + "*" descendant of this node rather than on the + // node, so look for it explicitly. + bool HasWildcardGrantForSelf() const { + const char pattern[2] = {node::kPathSeparator, '*'}; + size_t matched = 0; + const Node* current = this; + while (matched < 2) { + auto it = current->children.find(pattern[matched]); + if (it == current->children.end()) { + return false; + } + const Node* child = it->second; + for (size_t i = 0; i < child->prefix.length(); ++i) { + if (matched >= 2 || child->prefix[i] != pattern[matched]) { + return false; + } + ++matched; + } + current = child; + } + return current->wildcard_child != nullptr; + } }; RadixTree(); diff --git a/test/parallel/test-permission-fs-wildcard.js b/test/parallel/test-permission-fs-wildcard.js index 1b67f37c2dcda2..f6386ba0c92a45 100644 --- a/test/parallel/test-permission-fs-wildcard.js +++ b/test/parallel/test-permission-fs-wildcard.js @@ -126,3 +126,31 @@ if (common.isWindows) { assert.strictEqual(status, 0, stderr.toString()); } } + +{ + // A trailing-wildcard grant for a subdirectory must not leak access to an + // unrelated parent directory that only shares a path prefix with sibling + // grants. The directory that owns the wildcard stays accessible. + if (!common.isWindows) { + const { status, stderr } = spawnSync( + process.execPath, + [ + '--permission', + '--allow-fs-read=/etc/app/logs/*', + '--allow-fs-read=/etcd/data/file', + '--allow-fs-read=/etcx/y', + '-e', + ` + const assert = require('assert') + assert.ok(!process.permission.has('fs.read', '/etc')); + assert.ok(!process.permission.has('fs.read', '/etc/passwd')); + assert.ok(process.permission.has('fs.read', '/etc/app/logs')); + assert.ok(process.permission.has('fs.read', '/etc/app/logs/app.log')); + assert.ok(process.permission.has('fs.read', '/etcd/data/file')); + assert.ok(process.permission.has('fs.read', '/etcx/y')); + `, + ] + ); + assert.strictEqual(status, 0, stderr.toString()); + } +}