FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

permission: enforce fs write permission for trace events · nodejs/node@3cb607d · GitHub

/ node Public

Commit 3cb607d

Browse files
RafaelGSS
committed
permission: enforce fs write permission for trace events
Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com> PR-URL: nodejs-private/node-private#924 Refs: https://hackerone.com/reports/3838601 CVE-ID: CVE-2026-56847
1 parent 955e669 commit 3cb607d

4 files changed

Lines changed: 90 additions & 13 deletions

File tree

‎src/node_trace_events.cc‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@
55
#include "node_external_reference.h"
66
#include "node_internals.h"
77
#include "node_v8_platform-inl.h"
8+
#include "permission/permission.h"
89
#include "tracing/agent.h"
10+
#include "tracing/node_trace_writer.h"
911
#include "util-inl.h"
1012

1113
#include <set>
@@ -85,6 +87,11 @@ void NodeCategorySet::Enable(const FunctionCallbackInfo<Value>& args) {
8587
if (!category_set->enabled_ && !categories.empty()) {
8688
// Starts the Tracing Agent if it wasn't started already (e.g. through
8789
// a command line flag.)
90+
THROW_IF_INSUFFICIENT_PERMISSIONS(
91+
category_set->env(),
92+
permission::PermissionScope::kFileSystemWrite,
93+
tracing::NodeTraceWriter::GetFilePath(
94+
per_process::cli_options->trace_event_file_pattern, 1));
8895
auto* agent = tracing::Agent::GetInstance();
8996
agent->StartTracing(per_process::cli_options->trace_event_categories);
9097
tracing::AgentWriterHandle* writer = agent->GetDefaultWriterHandle();

‎src/tracing/node_trace_writer.cc‎

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,27 @@
88
namespace node {
99
namespace tracing {
1010

11+
void replace_substring(std::string* target,
12+
const std::string& search,
13+
const std::string& insert) {
14+
size_t pos = target->find(search);
15+
for (; pos != std::string::npos; pos = target->find(search, pos)) {
16+
target->replace(pos, search.size(), insert);
17+
pos += insert.size();
18+
}
19+
}
20+
1121
NodeTraceWriter::NodeTraceWriter(const std::string& log_file_pattern)
1222
: log_file_pattern_(log_file_pattern) {}
1323

24+
std::string NodeTraceWriter::GetFilePath(const std::string& log_file_pattern,
25+
int file_num) {
26+
std::string filepath(log_file_pattern);
27+
replace_substring(&filepath, "${pid}", std::to_string(uv_os_getpid()));
28+
replace_substring(&filepath, "${rotation}", std::to_string(file_num));
29+
return filepath;
30+
}
31+
1432
void NodeTraceWriter::InitializeOnThread(uv_loop_t* loop) {
1533
CHECK_NULL(tracing_loop_);
1634
tracing_loop_ = loop;
@@ -60,25 +78,13 @@ NodeTraceWriter::~NodeTraceWriter() {
6078
}
6179
}
6280

63-
void replace_substring(std::string* target,
64-
const std::string& search,
65-
const std::string& insert) {
66-
size_t pos = target->find(search);
67-
for (; pos != std::string::npos; pos = target->find(search, pos)) {
68-
target->replace(pos, search.size(), insert);
69-
pos += insert.size();
70-
}
71-
}
72-
7381
void NodeTraceWriter::OpenNewFileForStreaming() {
7482
++file_num_;
7583
uv_fs_t req;
7684

7785
// Evaluate a JS-style template string, it accepts the values ${pid} and
7886
// ${rotation}
79-
std::string filepath(log_file_pattern_);
80-
replace_substring(&filepath, "${pid}", std::to_string(uv_os_getpid()));
81-
replace_substring(&filepath, "${rotation}", std::to_string(file_num_));
87+
std::string filepath(GetFilePath(log_file_pattern_, file_num_));
8288

8389
if (fd_ != -1) {
8490
CHECK_EQ(uv_fs_close(nullptr, &req, fd_, nullptr), 0);

‎src/tracing/node_trace_writer.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@ class NodeTraceWriter : public AsyncTraceWriter {
2121
explicit NodeTraceWriter(const std::string& log_file_pattern);
2222
~NodeTraceWriter() override;
2323

24+
static std::string GetFilePath(const std::string& log_file_pattern,
25+
int file_num);
26+
2427
void InitializeOnThread(uv_loop_t* loop) override;
2528
void AppendTraceEvent(TraceObject* trace_event) override;
2629
void Flush(bool blocking) override;

‎test/parallel/test-permission-fs-write-trace-events.js‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
// Flags: --expose-internals
2+
'use strict';
3+
4+
const common = require('../common');
5+
const { spawnSyncAndExitWithoutError } = require('../common/child_process');
6+
const { isMainThread } = require('worker_threads');
7+
8+
if (!isMainThread) {
9+
common.skip('This test only works on a main thread');
10+
}
11+
12+
const assert = require('assert');
13+
const fs = require('fs');
14+
const tmpdir = require('../common/tmpdir');
15+
16+
try {
17+
require('trace_events');
18+
} catch {
19+
common.skip('missing trace events');
20+
}
21+
22+
if (!process.permission) {
23+
tmpdir.refresh();
24+
25+
const allowed = tmpdir.resolve('allowed');
26+
const outside = tmpdir.resolve('outside');
27+
fs.mkdirSync(allowed);
28+
fs.mkdirSync(outside);
29+
30+
spawnSyncAndExitWithoutError(process.execPath, [
31+
'--permission',
32+
'--allow-fs-read=*',
33+
`--allow-fs-write=${allowed}`,
34+
__filename,
35+
'child',
36+
], { cwd: outside });
37+
return;
38+
}
39+
40+
assert.strictEqual(process.argv[2], 'child');
41+
42+
assert.throws(() => {
43+
fs.writeFileSync('canary', 'x');
44+
}, common.expectsError({
45+
code: 'ERR_ACCESS_DENIED',
46+
permission: 'FileSystemWrite',
47+
}));
48+
49+
const tracing = require('trace_events').createTracing({
50+
categories: ['node', 'v8', 'node.perf'],
51+
});
52+
53+
assert.throws(() => {
54+
tracing.enable();
55+
}, common.expectsError({
56+
code: 'ERR_ACCESS_DENIED',
57+
permission: 'FileSystemWrite',
58+
resource: 'node_trace.1.log',
59+
}));
60+
61+
assert.strictEqual(fs.existsSync('node_trace.1.log'), false);

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL