Skip to content

Commit

Permalink
process: passing -1 to setuid/setgid should not abort
Browse files Browse the repository at this point in the history
Fixes: #32750
Signed-off-by: James M Snell <jasnell@gmail.com>

PR-URL: #36786
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Harshitha K P <harshitha014@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
  • Loading branch information
jasnell authored and danielleadams committed Jan 12, 2021
1 parent 1a4d34e commit 94afc3e
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 0 deletions.
1 change: 1 addition & 0 deletions lib/internal/bootstrap/switches/does_own_process_state.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ function wrapPosixCredentialSetters(credentials) {
function wrapIdSetter(type, method) {
return function(id) {
validateId(id, 'id');
if (typeof id === 'number') id |= 0;
// Result is 0 on success, 1 if credential is unknown.
const result = method(id);
if (result === 1) {
Expand Down
8 changes: 8 additions & 0 deletions test/parallel/test-process-uid-gid.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,13 @@ assert.throws(() => {
message: 'User identifier does not exist: fhqwhgadshgnsdhjsdbkhsdabkfabkveyb'
});

// Passing -0 shouldn't crash the process
// Refs: https://github.com/nodejs/node/issues/32750
try { process.setuid(-0); } catch {}
try { process.seteuid(-0); } catch {}
try { process.setgid(-0); } catch {}
try { process.setegid(-0); } catch {}

// If we're not running as super user...
if (process.getuid() !== 0) {
// Should not throw.
Expand Down Expand Up @@ -79,6 +86,7 @@ try {
}
process.setgid('nogroup');
}

const newgid = process.getgid();
assert.notStrictEqual(newgid, oldgid);

Expand Down

0 comments on commit 94afc3e

Please sign in to comment.