From f4333b1cdddb270db368e0a649ace60ea7e8d53d Mon Sep 17 00:00:00 2001 From: Rafael Gonzaga Date: Tue, 4 Jul 2023 10:12:48 -0300 Subject: [PATCH] permission: v8.writeHeapSnapshot and process.report Co-Authored-By: haxatron Signed-off-by: RafaelGSS PR-URL: https://github.com/nodejs/node/pull/48564 Reviewed-By: Paolo Insogna Reviewed-By: Marco Ippolito --- lib/internal/process/report.js | 2 ++ src/heap_utils.cc | 5 ++++ src/node_report.cc | 10 +++++++ .../test-permission-fs-write-report.js | 30 +++++++++++++++++++ test/parallel/test-permission-fs-write-v8.js | 30 +++++++++++++++++++ 5 files changed, 77 insertions(+) create mode 100644 test/parallel/test-permission-fs-write-report.js create mode 100644 test/parallel/test-permission-fs-write-v8.js diff --git a/lib/internal/process/report.js b/lib/internal/process/report.js index c83371dcfcf938..9889f913c3f81f 100644 --- a/lib/internal/process/report.js +++ b/lib/internal/process/report.js @@ -2,6 +2,7 @@ const { ERR_SYNTHETIC, } = require('internal/errors').codes; +const { getValidatedPath } = require('internal/fs/utils'); const { validateBoolean, validateObject, @@ -19,6 +20,7 @@ const report = { file = undefined; } else if (file !== undefined) { validateString(file, 'file'); + file = getValidatedPath(file); } if (err === undefined) { diff --git a/src/heap_utils.cc b/src/heap_utils.cc index 09558fad3fd226..b423d97345e956 100644 --- a/src/heap_utils.cc +++ b/src/heap_utils.cc @@ -2,6 +2,7 @@ #include "env-inl.h" #include "memory_tracker-inl.h" #include "node_external_reference.h" +#include "permission/permission.h" #include "stream_base-inl.h" #include "util-inl.h" @@ -454,6 +455,8 @@ void TriggerHeapSnapshot(const FunctionCallbackInfo& args) { if (filename_v->IsUndefined()) { DiagnosticFilename name(env, "Heap", "heapsnapshot"); + THROW_IF_INSUFFICIENT_PERMISSIONS( + env, permission::PermissionScope::kFileSystemWrite, env->GetCwd()); if (WriteSnapshot(env, *name, options).IsNothing()) return; if (String::NewFromUtf8(isolate, *name).ToLocal(&filename_v)) { args.GetReturnValue().Set(filename_v); @@ -463,6 +466,8 @@ void TriggerHeapSnapshot(const FunctionCallbackInfo& args) { BufferValue path(isolate, filename_v); CHECK_NOT_NULL(*path); + THROW_IF_INSUFFICIENT_PERMISSIONS( + env, permission::PermissionScope::kFileSystemWrite, path.ToStringView()); if (WriteSnapshot(env, *path, options).IsNothing()) return; return args.GetReturnValue().Set(filename_v); } diff --git a/src/node_report.cc b/src/node_report.cc index 7c1c9e66d29833..dcaa6922070b92 100644 --- a/src/node_report.cc +++ b/src/node_report.cc @@ -7,6 +7,7 @@ #include "node_metadata.h" #include "node_mutex.h" #include "node_worker.h" +#include "permission/permission.h" #include "util.h" #ifdef _WIN32 @@ -856,6 +857,8 @@ std::string TriggerNodeReport(Isolate* isolate, // Determine the required report filename. In order of priority: // 1) supplied on API 2) configured on startup 3) default generated if (!name.empty()) { + THROW_IF_INSUFFICIENT_PERMISSIONS( + env, permission::PermissionScope::kFileSystemWrite, name, name); // Filename was specified as API parameter. filename = name; } else { @@ -871,6 +874,13 @@ std::string TriggerNodeReport(Isolate* isolate, filename = *DiagnosticFilename( env != nullptr ? env->thread_id() : 0, "report", "json"); } + if (env != nullptr) { + THROW_IF_INSUFFICIENT_PERMISSIONS( + env, + permission::PermissionScope::kFileSystemWrite, + std::string_view(env->GetCwd()), + filename); + } } // Open the report file stream for writing. Supports stdout/err, diff --git a/test/parallel/test-permission-fs-write-report.js b/test/parallel/test-permission-fs-write-report.js new file mode 100644 index 00000000000000..a66bd316d02439 --- /dev/null +++ b/test/parallel/test-permission-fs-write-report.js @@ -0,0 +1,30 @@ +// Flags: --experimental-permission --allow-fs-read=* +'use strict'; + +const common = require('../common'); +common.skipIfWorker(); +if (!common.hasCrypto) + common.skip('no crypto'); + +const assert = require('assert'); +const path = require('path'); + +{ + assert.throws(() => { + process.report.writeReport('./secret.txt'); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.resolve('./secret.txt'), + })); +} + +{ + assert.throws(() => { + process.report.writeReport(); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: process.cwd(), + })); +} diff --git a/test/parallel/test-permission-fs-write-v8.js b/test/parallel/test-permission-fs-write-v8.js new file mode 100644 index 00000000000000..201c03e067b575 --- /dev/null +++ b/test/parallel/test-permission-fs-write-v8.js @@ -0,0 +1,30 @@ +// Flags: --experimental-permission --allow-fs-read=* +'use strict'; + +const common = require('../common'); +common.skipIfWorker(); +if (!common.hasCrypto) + common.skip('no crypto'); + +const assert = require('assert'); +const v8 = require('v8'); +const path = require('path'); + +{ + assert.throws(() => { + v8.writeHeapSnapshot('./secret.txt'); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(path.resolve('./secret.txt')), + })); +} + +{ + assert.throws(() => { + v8.writeHeapSnapshot(); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + })); +}