Skip to content

Commit 7d33dac

Browse files
committed
permission: fix parent directory over-grant in fs radix tree
1 parent ab41cf0 commit 7d33dac

3 files changed

Lines changed: 65 additions & 2 deletions

File tree

src/permission/fs_permission.cc

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,11 @@ bool FSPermission::RadixTree::Lookup(const std::string_view& s,
304304

305305
auto node = current_node->NextNode(path, parent_node_prefix_len);
306306
if (node == nullptr) {
307-
return false;
307+
// The whole path matched this node. Grant it when the directory itself
308+
// was allowed through a trailing wildcard ("/dir/*"), which after a
309+
// radix split lives on a descendant rather than on this node.
310+
return parent_node_prefix_len == path_len &&
311+
current_node->HasWildcardGrantForSelf();
308312
}
309313

310314
current_node = node;

src/permission/fs_permission.h

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,13 @@ class FSPermission final : public PermissionBase {
6868
return split_child->CreateChild(path_prefix.substr(i));
6969
}
7070
}
71-
child->is_leaf = true;
71+
// Only mark the child as a leaf when it already terminates an
72+
// inserted path. Routing a longer path through this node must not
73+
// turn a pass-through prefix into a granted entry, or an unrelated
74+
// parent directory would be treated as allowed.
75+
if (child->IsEndNode()) {
76+
child->is_leaf = true;
77+
}
7278
return child->CreateChild(path_prefix.substr(i));
7379
}
7480

@@ -137,6 +143,31 @@ class FSPermission final : public PermissionBase {
137143
}
138144
return is_leaf;
139145
}
146+
147+
// A directory allowed with a trailing wildcard ("/dir/*") also grants
148+
// access to the directory itself. After a radix split that grant is
149+
// stored as a "/" + "*" descendant of this node rather than on the
150+
// node, so look for it explicitly.
151+
bool HasWildcardGrantForSelf() const {
152+
const char pattern[2] = {node::kPathSeparator, '*'};
153+
size_t matched = 0;
154+
const Node* current = this;
155+
while (matched < 2) {
156+
auto it = current->children.find(pattern[matched]);
157+
if (it == current->children.end()) {
158+
return false;
159+
}
160+
const Node* child = it->second;
161+
for (size_t i = 0; i < child->prefix.length(); ++i) {
162+
if (matched >= 2 || child->prefix[i] != pattern[matched]) {
163+
return false;
164+
}
165+
++matched;
166+
}
167+
current = child;
168+
}
169+
return current->wildcard_child != nullptr;
170+
}
140171
};
141172

142173
RadixTree();

test/parallel/test-permission-fs-wildcard.js

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,3 +126,31 @@ if (common.isWindows) {
126126
assert.strictEqual(status, 0, stderr.toString());
127127
}
128128
}
129+
130+
{
131+
// A trailing-wildcard grant for a subdirectory must not leak access to an
132+
// unrelated parent directory that only shares a path prefix with sibling
133+
// grants. The directory that owns the wildcard stays accessible.
134+
if (!common.isWindows) {
135+
const { status, stderr } = spawnSync(
136+
process.execPath,
137+
[
138+
'--permission',
139+
'--allow-fs-read=/etc/app/logs/*',
140+
'--allow-fs-read=/etcd/data/file',
141+
'--allow-fs-read=/etcx/y',
142+
'-e',
143+
`
144+
const assert = require('assert')
145+
assert.ok(!process.permission.has('fs.read', '/etc'));
146+
assert.ok(!process.permission.has('fs.read', '/etc/passwd'));
147+
assert.ok(process.permission.has('fs.read', '/etc/app/logs'));
148+
assert.ok(process.permission.has('fs.read', '/etc/app/logs/app.log'));
149+
assert.ok(process.permission.has('fs.read', '/etcd/data/file'));
150+
assert.ok(process.permission.has('fs.read', '/etcx/y'));
151+
`,
152+
]
153+
);
154+
assert.strictEqual(status, 0, stderr.toString());
155+
}
156+
}

0 commit comments

Comments
 (0)