Skip to content
Permalink
Browse files
process: fix uid/gid validation to avoid crash
id |= 0 turns unsigned 32-bit integer values exceeding the unsigned
31-bit range into negative integers, causing a crash. Use id >>>= 0
instead, which works properly for all unsigned 32-bit integers.

Refs: #36786
PR-URL: #44910
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Erick Wendel <erick.workspace@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
  • Loading branch information
tniessen authored and danielleadams committed Oct 11, 2022
1 parent 9f2dd48 commit 803fbfb168d219fa5741362e1f6467a7702bfc58
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 12 deletions.
@@ -76,7 +76,7 @@ function wrapPosixCredentialSetters(credentials) {
function wrapIdSetter(type, method) {
return function(id) {
validateId(id, 'id');
if (typeof id === 'number') id |= 0;
if (typeof id === 'number') id >>>= 0;
// Result is 0 on success, 1 if credential is unknown.
const result = method(id);
if (result === 1) {
@@ -53,17 +53,13 @@ assert.throws(() => {

// Passing -0 shouldn't crash the process
// Refs: https://github.com/nodejs/node/issues/32750
try { process.setuid(-0); } catch {
// Continue regardless of error.
}
try { process.seteuid(-0); } catch {
// Continue regardless of error.
}
try { process.setgid(-0); } catch {
// Continue regardless of error.
}
try { process.setegid(-0); } catch {
// Continue regardless of error.
// And neither should values exceeding 2 ** 31 - 1.
for (const id of [-0, 2 ** 31, 2 ** 32 - 1]) {
for (const fn of [process.setuid, process.setuid, process.setgid, process.setegid]) {
try { fn(id); } catch {
// Continue regardless of error.
}
}
}

// If we're not running as super user...

0 comments on commit 803fbfb

Please sign in to comment.