Skip to content

Commit ed18b9c

Browse files
committed
permission: check final report output path
Refs: https://hackerone.com/reports/3815767 Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com> PR-URL: nodejs-private/node-private#926 CVE-ID: CVE-2026-58039
1 parent acaf426 commit ed18b9c

3 files changed

Lines changed: 86 additions & 11 deletions

File tree

lib/internal/process/report.js

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ const {
99
ERR_SYNTHETIC,
1010
} = require('internal/errors').codes;
1111
const { getValidatedPath } = require('internal/fs/utils');
12+
const { sep } = require('path');
1213
const permission = require('internal/process/permission');
1314
const {
1415
validateBoolean,
@@ -29,7 +30,14 @@ const report = {
2930
}
3031

3132
if (permission.isEnabled()) {
32-
const resource = file ?? process.cwd();
33+
let resource = file;
34+
if (resource !== undefined) {
35+
const directory = nr.getDirectory();
36+
if (directory !== '')
37+
resource = `${directory}${sep}${resource}`;
38+
} else {
39+
resource = process.cwd();
40+
}
3341
if (!permission.has('fs.write', resource)) {
3442
throw new ERR_ACCESS_DENIED(
3543
'Access to this API has been restricted',

src/node_report.cc

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -851,13 +851,6 @@ std::string TriggerNodeReport(Isolate* isolate,
851851
filename = *DiagnosticFilename(
852852
env != nullptr ? env->thread_id() : 0, "report", "json");
853853
}
854-
if (env != nullptr) {
855-
THROW_IF_INSUFFICIENT_PERMISSIONS(
856-
env,
857-
permission::PermissionScope::kFileSystemWrite,
858-
Environment::GetCwd(env->exec_path()),
859-
filename);
860-
}
861854
}
862855

863856
// Open the report file stream for writing. Supports stdout/err,
@@ -875,12 +868,21 @@ std::string TriggerNodeReport(Isolate* isolate,
875868
report_directory = per_process::cli_options->report_directory;
876869
}
877870
// Regular file. Append filename to directory path if one was specified
871+
std::string pathname;
878872
if (report_directory.length() > 0) {
879-
std::string pathname = report_directory + kPathSeparator + filename;
880-
outfile.open(pathname, std::ios::out | std::ios::binary);
873+
pathname = report_directory + kPathSeparator + filename;
881874
} else {
882-
outfile.open(filename, std::ios::out | std::ios::binary);
875+
pathname = filename;
876+
}
877+
878+
// We may not always be in a great state when generating a node report.
879+
// Allow for the case where we don't have an env.
880+
if (env != nullptr) {
881+
THROW_IF_INSUFFICIENT_PERMISSIONS(
882+
env, permission::PermissionScope::kFileSystemWrite, pathname, "");
883883
}
884+
885+
outfile.open(pathname, std::ios::out | std::ios::binary);
884886
// Check for errors on the file open
885887
if (!outfile.is_open()) {
886888
std::cerr << "\nFailed to open Node.js report file: " << filename;

test/parallel/test-permission-fs-write-report.js

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ if (!process.permission) {
2323
}
2424

2525
const assert = require('assert');
26+
const fs = require('fs');
2627
const path = require('path');
2728
const tmpdir = require('../common/tmpdir');
2829

@@ -73,3 +74,67 @@ spawnSyncAndExitWithoutError(
7374
],
7475
{ cwd: tmpdir.path }
7576
);
77+
78+
{
79+
const allowedDir = path.join(tmpdir.path, 'report-allowed');
80+
const deniedDir = path.join(tmpdir.path, 'report-denied');
81+
fs.mkdirSync(allowedDir);
82+
fs.mkdirSync(deniedDir);
83+
84+
const deniedFile = path.join(deniedDir, 'report.json');
85+
fs.writeFileSync(deniedFile, 'existing content');
86+
spawnSyncAndExitWithoutError(
87+
process.execPath,
88+
[
89+
'--permission',
90+
'--allow-fs-read=*',
91+
`--allow-fs-write=${allowedDir}`,
92+
'-e',
93+
`
94+
const assert = require('assert');
95+
process.report.directory = ${JSON.stringify(deniedDir)};
96+
assert.throws(() => {
97+
process.report.writeReport('report.json');
98+
}, {
99+
code: 'ERR_ACCESS_DENIED',
100+
permission: 'FileSystemWrite',
101+
resource: ${JSON.stringify(deniedFile)},
102+
});
103+
`,
104+
],
105+
{ cwd: allowedDir },
106+
);
107+
assert.strictEqual(fs.readFileSync(deniedFile, 'utf8'), 'existing content');
108+
}
109+
110+
{
111+
const allowedDir = path.join(tmpdir.path, 'report-filename-allowed');
112+
const deniedDir = path.join(tmpdir.path, 'report-filename-denied');
113+
fs.mkdirSync(allowedDir);
114+
fs.mkdirSync(deniedDir);
115+
116+
const deniedFile = path.join(deniedDir, 'report.json');
117+
spawnSyncAndExitWithoutError(
118+
process.execPath,
119+
[
120+
'--permission',
121+
'--allow-fs-read=*',
122+
`--allow-fs-write=${allowedDir}`,
123+
'-e',
124+
`
125+
const assert = require('assert');
126+
process.report.directory = ${JSON.stringify(deniedDir)};
127+
process.report.filename = 'report.json';
128+
assert.throws(() => {
129+
process.report.writeReport();
130+
}, {
131+
code: 'ERR_ACCESS_DENIED',
132+
permission: 'FileSystemWrite',
133+
resource: ${JSON.stringify(deniedFile)},
134+
});
135+
`,
136+
],
137+
{ cwd: allowedDir },
138+
);
139+
assert.strictEqual(fs.existsSync(deniedFile), false);
140+
}

0 commit comments

Comments
 (0)