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

src: pass resource on permission checks for spawn · nodejs/node@a0b1378 · GitHub

/ node Public

Commit a0b1378

Browse files
RafaelGSS
committed
src: pass resource on permission checks for spawn
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: #58758 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Edy Silva <edigleyssonsilva@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
1 parent 081c708 commit a0b1378

5 files changed

Lines changed: 40 additions & 18 deletions

File tree

‎src/node_process_methods.cc‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -518,9 +518,6 @@ static void Execve(const FunctionCallbackInfo<Value>& args) {
518518
Isolate* isolate = env->isolate();
519519
Local<Context> context = env->context();
520520

521-
THROW_IF_INSUFFICIENT_PERMISSIONS(
522-
env, permission::PermissionScope::kChildProcess, "");
523-
524521
CHECK(args[0]->IsString());
525522
CHECK(args[1]->IsArray());
526523
CHECK(args[2]->IsArray());
@@ -530,6 +527,11 @@ static void Execve(const FunctionCallbackInfo<Value>& args) {
530527

531528
// Copy arguments and environment
532529
Utf8Value executable(isolate, args[0]);
530+
531+
THROW_IF_INSUFFICIENT_PERMISSIONS(env,
532+
permission::PermissionScope::kChildProcess,
533+
executable.ToStringView());
534+
533535
std::vector<std::string> argv_strings(argv_array->Length());
534536
std::vector<std::string> envp_strings(envp_array->Length());
535537
std::vector<char*> argv(argv_array->Length() + 1);

‎src/process_wrap.cc‎

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -186,8 +186,6 @@ class ProcessWrap : public HandleWrap {
186186
Local<Context> context = env->context();
187187
ProcessWrap* wrap;
188188
ASSIGN_OR_RETURN_UNWRAP(&wrap, args.This());
189-
THROW_IF_INSUFFICIENT_PERMISSIONS(
190-
env, permission::PermissionScope::kChildProcess, "");
191189
int err = 0;
192190

193191
if (!args[0]->IsObject()) {
@@ -201,6 +199,20 @@ class ProcessWrap : public HandleWrap {
201199

202200
options.exit_cb = OnExit;
203201

202+
// TODO(bnoordhuis) is this possible to do without mallocing ?
203+
204+
// options.file
205+
Local<Value> file_v;
206+
if (!js_options->Get(context, env->file_string()).ToLocal(&file_v)) {
207+
return;
208+
}
209+
CHECK(file_v->IsString());
210+
node::Utf8Value file(env->isolate(), file_v);
211+
options.file = *file;
212+
213+
THROW_IF_INSUFFICIENT_PERMISSIONS(
214+
env, permission::PermissionScope::kChildProcess, file.ToStringView());
215+
204216
// options.uid
205217
Local<Value> uid_v;
206218
if (!js_options->Get(context, env->uid_string()).ToLocal(&uid_v)) {
@@ -225,17 +237,6 @@ class ProcessWrap : public HandleWrap {
225237
options.gid = static_cast<uv_gid_t>(gid);
226238
}
227239

228-
// TODO(bnoordhuis) is this possible to do without mallocing ?
229-
230-
// options.file
231-
Local<Value> file_v;
232-
if (!js_options->Get(context, env->file_string()).ToLocal(&file_v)) {
233-
return;
234-
}
235-
CHECK(file_v->IsString());
236-
node::Utf8Value file(env->isolate(), file_v);
237-
options.file = *file;
238-
239240
// Undocumented feature of Win32 CreateProcess API allows spawning
240241
// batch files directly but is potentially insecure because arguments
241242
// are not escaped (and sometimes cannot be unambiguously escaped),

‎src/spawn_sync.cc‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -378,8 +378,24 @@ void SyncProcessRunner::RegisterExternalReferences(
378378

379379
void SyncProcessRunner::Spawn(const FunctionCallbackInfo<Value>& args) {
380380
Environment* env = Environment::GetCurrent(args);
381+
Local<Context> context = env->context();
382+
383+
std::string resource = "";
384+
if (env->permission()->enabled() && args[0]->IsObject()) {
385+
Local<Object> js_options = args[0].As<Object>();
386+
Local<Value> js_file;
387+
if (!js_options->Get(context, env->file_string()).ToLocal(&js_file)) {
388+
return;
389+
}
390+
391+
if (js_file->IsString()) {
392+
node::Utf8Value executable(env->isolate(), js_file.As<String>());
393+
resource = executable.ToString();
394+
}
395+
}
396+
381397
THROW_IF_INSUFFICIENT_PERMISSIONS(
382-
env, permission::PermissionScope::kChildProcess, "");
398+
env, permission::PermissionScope::kChildProcess, resource);
383399
env->PrintSyncTrace();
384400
SyncProcessRunner p(env);
385401
Local<Value> result;

‎test/parallel/test-permission-child-process-cli.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,14 @@ if (process.argv[2] === 'child') {
2929
message: 'Access to this API has been restricted. Use --allow-child-process to manage permissions.',
3030
code: 'ERR_ACCESS_DENIED',
3131
permission: 'ChildProcess',
32+
resource: process.execPath,
3233
}));
3334
assert.throws(() => {
3435
childProcess.spawnSync(process.execPath, ['--version']);
3536
}, common.expectsError({
3637
code: 'ERR_ACCESS_DENIED',
3738
permission: 'ChildProcess',
39+
resource: process.execPath,
3840
}));
3941
assert.throws(() => {
4042
childProcess.exec(...common.escapePOSIXShell`"${process.execPath}" --version`);
@@ -53,6 +55,7 @@ if (process.argv[2] === 'child') {
5355
}, common.expectsError({
5456
code: 'ERR_ACCESS_DENIED',
5557
permission: 'ChildProcess',
58+
resource: process.execPath,
5659
}));
5760
assert.throws(() => {
5861
childProcess.execFile(...common.escapePOSIXShell`"${process.execPath}" --version`);

‎test/parallel/test-process-execve-permission-fail.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,5 +17,5 @@ if (process.argv[2] === 'replaced') {
1717
} else {
1818
throws(mustCall(() => {
1919
process.execve(process.execPath, [process.execPath, __filename, 'replaced'], process.env);
20-
}), { code: 'ERR_ACCESS_DENIED', permission: 'ChildProcess' });
20+
}), { code: 'ERR_ACCESS_DENIED', permission: 'ChildProcess', resource: process.execPath });
2121
}

0 commit comments

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