Skip to content

Commit ad99d07

Browse files
committed
permission: avoid granting radix split nodes
Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com> PR-URL: nodejs-private/node-private#911 Refs: https://hackerone.com/reports/3761342 CVE-ID: CVE-2026-58043
1 parent dbeeaee commit ad99d07

2 files changed

Lines changed: 51 additions & 2 deletions

File tree

src/permission/fs_permission.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,12 +51,13 @@ class FSPermission final : public PermissionBase {
5151
children[label] = new Node(path_prefix);
5252
return children[label];
5353
}
54+
bool child_was_end_node = child->IsEndNode();
5455

5556
// swap prefix
5657
size_t i = 0;
5758
size_t prefix_len = path_prefix.length();
5859
for (; i < child->prefix.length(); ++i) {
59-
if (i > prefix_len || path_prefix[i] != child->prefix[i]) {
60+
if (i >= prefix_len || path_prefix[i] != child->prefix[i]) {
6061
std::string parent_prefix = child->prefix.substr(0, i);
6162
std::string child_prefix = child->prefix.substr(i);
6263

@@ -68,7 +69,9 @@ class FSPermission final : public PermissionBase {
6869
return split_child->CreateChild(path_prefix.substr(i));
6970
}
7071
}
71-
child->is_leaf = true;
72+
if (child_was_end_node) {
73+
child->is_leaf = true;
74+
}
7275
return child->CreateChild(path_prefix.substr(i));
7376
}
7477

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

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ if (!common.hasCrypto) {
1515
const assert = require('assert');
1616
const fixtures = require('../common/fixtures');
1717
const tmpdir = require('../common/tmpdir');
18+
const fs = require('fs');
1819
const { spawnSync } = require('child_process');
1920
const path = require('path');
2021

@@ -28,6 +29,51 @@ const commonPath = path.join(__filename, '../../common');
2829
tmpdir.refresh();
2930
}
3031

32+
{
33+
const boundaryFile = path.join(tmpdir.path, 'secret');
34+
const grantedFiles = ['secret1', 'secret2', 'secret3']
35+
.map((file) => path.join(tmpdir.path, file));
36+
37+
fs.writeFileSync(boundaryFile, 'protected');
38+
for (const file of grantedFiles) {
39+
fs.writeFileSync(file, 'granted');
40+
}
41+
42+
const { status, stderr } = spawnSync(
43+
process.execPath,
44+
[
45+
'--permission',
46+
...grantedFiles.map((file) => `--allow-fs-read=${file}`),
47+
...grantedFiles.map((file) => `--allow-fs-write=${file}`),
48+
'-e',
49+
`
50+
const assert = require('assert');
51+
const fs = require('fs');
52+
const target = process.env.BOUNDARY_FILE;
53+
54+
assert.strictEqual(process.permission.has('fs.read', target), false);
55+
assert.strictEqual(process.permission.has('fs.write', target), false);
56+
assert.throws(
57+
() => fs.readFileSync(target, 'utf8'),
58+
{ code: 'ERR_ACCESS_DENIED', permission: 'FileSystemRead' }
59+
);
60+
assert.throws(
61+
() => fs.writeFileSync(target, 'modified'),
62+
{ code: 'ERR_ACCESS_DENIED', permission: 'FileSystemWrite' }
63+
);
64+
`,
65+
],
66+
{
67+
env: {
68+
...process.env,
69+
BOUNDARY_FILE: boundaryFile,
70+
},
71+
}
72+
);
73+
assert.strictEqual(status, 0, stderr.toString());
74+
assert.strictEqual(fs.readFileSync(boundaryFile, 'utf8'), 'protected');
75+
}
76+
3177
{
3278
const { status, stderr } = spawnSync(
3379
process.execPath,

0 commit comments

Comments
 (0)