OSSForks/node
1
0
Fork 0
mirror of https://github.com/zebrajr/node.git synced 2025-12-06 00:20:08 +01:00
Code Issues Actions 8 Packages Projects Releases Wiki Activity

src: pass resource on permission checks for spawn

Browse Source 
This commit enhances the permission model errors
when no --allow-child-process is used and the error
is emitted.

Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com>
PR-URL: https://github.com/nodejs/node/pull/58758
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Edy Silva <edigleyssonsilva@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
This commit is contained in:
Rafael Gonzaga 2025-06-21 11:40:10 -03:00 committed by GitHub
parent e679e38b25
commit b04c4a44a5
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 40 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
src/node_process_methods.cc
View File

@ -518,9 +518,6 @@ static void Execve(const FunctionCallbackInfo<Value>& args) {
Isolate* isolate = env->isolate();
Local<Context> context = env->context();
THROW_IF_INSUFFICIENT_PERMISSIONS(
env, permission::PermissionScope::kChildProcess, "");
CHECK(args[0]->IsString());
CHECK(args[1]->IsArray());
CHECK(args[2]->IsArray());
@ -530,6 +527,11 @@ static void Execve(const FunctionCallbackInfo<Value>& args) {
// Copy arguments and environment
Utf8Value executable(isolate, args[0]);
THROW_IF_INSUFFICIENT_PERMISSIONS(env,
permission::PermissionScope::kChildProcess,
executable.ToStringView());
std::vector<std::string> argv_strings(argv_array->Length());
std::vector<std::string> envp_strings(envp_array->Length());
std::vector<char*> argv(argv_array->Length() + 1);

27
src/process_wrap.cc
View File

@ -186,8 +186,6 @@ class ProcessWrap : public HandleWrap {
Local<Context> context = env->context();
ProcessWrap* wrap;
ASSIGN_OR_RETURN_UNWRAP(&wrap, args.This());
THROW_IF_INSUFFICIENT_PERMISSIONS(
env, permission::PermissionScope::kChildProcess, "");
int err = 0;
if (!args[0]->IsObject()) {
@ -201,6 +199,20 @@ class ProcessWrap : public HandleWrap {
options.exit_cb = OnExit;
// TODO(bnoordhuis) is this possible to do without mallocing ?
// options.file
Local<Value> file_v;
if (!js_options->Get(context, env->file_string()).ToLocal(&file_v)) {
return;
}
CHECK(file_v->IsString());
node::Utf8Value file(env->isolate(), file_v);
options.file = *file;
THROW_IF_INSUFFICIENT_PERMISSIONS(
env, permission::PermissionScope::kChildProcess, file.ToStringView());
// options.uid
Local<Value> uid_v;
if (!js_options->Get(context, env->uid_string()).ToLocal(&uid_v)) {
@ -225,17 +237,6 @@ class ProcessWrap : public HandleWrap {
options.gid = static_cast<uv_gid_t>(gid);
}
// TODO(bnoordhuis) is this possible to do without mallocing ?
// options.file
Local<Value> file_v;
if (!js_options->Get(context, env->file_string()).ToLocal(&file_v)) {
return;
}
CHECK(file_v->IsString());
node::Utf8Value file(env->isolate(), file_v);
options.file = *file;
// Undocumented feature of Win32 CreateProcess API allows spawning
// batch files directly but is potentially insecure because arguments
// are not escaped (and sometimes cannot be unambiguously escaped),

18
src/spawn_sync.cc
View File

@ -378,8 +378,24 @@ void SyncProcessRunner::RegisterExternalReferences(
void SyncProcessRunner::Spawn(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
Local<Context> context = env->context();
std::string resource = "";
if (env->permission()->enabled() && args[0]->IsObject()) {
Local<Object> js_options = args[0].As<Object>();
Local<Value> js_file;
if (!js_options->Get(context, env->file_string()).ToLocal(&js_file)) {
return;
}
if (js_file->IsString()) {
node::Utf8Value executable(env->isolate(), js_file.As<String>());
resource = executable.ToString();
}
}
THROW_IF_INSUFFICIENT_PERMISSIONS(
env, permission::PermissionScope::kChildProcess, "");
env, permission::PermissionScope::kChildProcess, resource);
env->PrintSyncTrace();
SyncProcessRunner p(env);
Local<Value> result;

3
test/parallel/test-permission-child-process-cli.js
View File

@ -29,12 +29,14 @@ if (process.argv[2] === 'child') {
message: 'Access to this API has been restricted. Use --allow-child-process to manage permissions.',
code: 'ERR_ACCESS_DENIED',
permission: 'ChildProcess',
resource: process.execPath,
}));
assert.throws(() => {
childProcess.spawnSync(process.execPath, ['--version']);
}, common.expectsError({
code: 'ERR_ACCESS_DENIED',
permission: 'ChildProcess',
resource: process.execPath,
}));
assert.throws(() => {
childProcess.exec(...common.escapePOSIXShell`"${process.execPath}" --version`);
@ -53,6 +55,7 @@ if (process.argv[2] === 'child') {
}, common.expectsError({
code: 'ERR_ACCESS_DENIED',
permission: 'ChildProcess',
resource: process.execPath,
}));
assert.throws(() => {
childProcess.execFile(...common.escapePOSIXShell`"${process.execPath}" --version`);

2
test/parallel/test-process-execve-permission-fail.js
View File

@ -17,5 +17,5 @@ if (process.argv[2] === 'replaced') {
} else {
throws(mustCall(() => {
process.execve(process.execPath, [process.execPath, __filename, 'replaced'], process.env);
}), { code: 'ERR_ACCESS_DENIED', permission: 'ChildProcess' });
}), { code: 'ERR_ACCESS_DENIED', permission: 'ChildProcess', resource: process.execPath });
}