From 30ecea94abe31f94e2c7591117d0bb8e713ffd47 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Mon, 4 Jan 2021 12:44:33 -0800 Subject: [PATCH 1/2] process: passing -1 to setuid/setgid should not abort Fixes: https://github.com/nodejs/node/issues/32750 Signed-off-by: James M Snell --- lib/internal/bootstrap/switches/does_own_process_state.js | 1 + test/parallel/test-process-uid-gid.js | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/lib/internal/bootstrap/switches/does_own_process_state.js b/lib/internal/bootstrap/switches/does_own_process_state.js index 5ee7f079d10124..c3ebf5537c60c9 100644 --- a/lib/internal/bootstrap/switches/does_own_process_state.js +++ b/lib/internal/bootstrap/switches/does_own_process_state.js @@ -81,6 +81,7 @@ function wrapPosixCredentialSetters(credentials) { return function(id) { validateId(id, 'id'); // Result is 0 on success, 1 if credential is unknown. + if (typeof id === 'number') id |= 0; const result = method(id); if (result === 1) { throw new ERR_UNKNOWN_CREDENTIAL(type, id); diff --git a/test/parallel/test-process-uid-gid.js b/test/parallel/test-process-uid-gid.js index 6ca2e009571ef0..0e170620b7f237 100644 --- a/test/parallel/test-process-uid-gid.js +++ b/test/parallel/test-process-uid-gid.js @@ -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. @@ -79,6 +86,7 @@ try { } process.setgid('nogroup'); } + const newgid = process.getgid(); assert.notStrictEqual(newgid, oldgid); From 02b9facc4c92f9ce154297f1c8af4207394a5758 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Tue, 5 Jan 2021 23:45:33 -0800 Subject: [PATCH 2/2] [Squash] ... MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Michaël Zasso --- lib/internal/bootstrap/switches/does_own_process_state.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/bootstrap/switches/does_own_process_state.js b/lib/internal/bootstrap/switches/does_own_process_state.js index c3ebf5537c60c9..0d60fb1f4595d1 100644 --- a/lib/internal/bootstrap/switches/does_own_process_state.js +++ b/lib/internal/bootstrap/switches/does_own_process_state.js @@ -80,8 +80,8 @@ function wrapPosixCredentialSetters(credentials) { function wrapIdSetter(type, method) { return function(id) { validateId(id, 'id'); - // Result is 0 on success, 1 if credential is unknown. if (typeof id === 'number') id |= 0; + // Result is 0 on success, 1 if credential is unknown. const result = method(id); if (result === 1) { throw new ERR_UNKNOWN_CREDENTIAL(type, id);