Skip to content

Commit 465244f

Browse files
j0t3xBridgeAR
authored andcommitted
lib,src: audit process.env in lib/ for setuid binary
Wrap SafeGetenv() in util binding with the purpose of protecting the cases when env vars are accessed with the privileges of another user in jsland. PR-URL: nodejs#18511 Fixes: nodejs#9160 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
1 parent 2008a8c commit 465244f

4 files changed

Lines changed: 33 additions & 7 deletions

File tree

lib/internal/modules/cjs/loader.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ const {
4040
stripBOM,
4141
stripShebang
4242
} = require('internal/modules/cjs/helpers');
43+
const { safeGetenv } = process.binding('util');
4344
const preserveSymlinks = !!process.binding('config').preserveSymlinks;
4445
const experimentalModules = !!process.binding('config').experimentalModules;
4546

@@ -701,10 +702,13 @@ Module._initPaths = function() {
701702
const isWindows = process.platform === 'win32';
702703

703704
var homeDir;
705+
var nodePath;
704706
if (isWindows) {
705707
homeDir = process.env.USERPROFILE;
708+
nodePath = process.env.NODE_PATH;
706709
} else {
707-
homeDir = process.env.HOME;
710+
homeDir = safeGetenv('HOME');
711+
nodePath = safeGetenv('NODE_PATH');
708712
}
709713

710714
// $PREFIX/lib/node, where $PREFIX is the root of the Node.js installation.
@@ -723,7 +727,6 @@ Module._initPaths = function() {
723727
paths.unshift(path.resolve(homeDir, '.node_modules'));
724728
}
725729

726-
var nodePath = process.env.NODE_PATH;
727730
if (nodePath) {
728731
paths = nodePath.split(path.delimiter).filter(function(path) {
729732
return !!path;

lib/os.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121

2222
'use strict';
2323

24-
const { pushValToArrayMax } = process.binding('util');
24+
const { pushValToArrayMax, safeGetenv } = process.binding('util');
2525
const constants = process.binding('constants').os;
2626
const { deprecate } = require('internal/util');
2727
const { getCIDRSuffix } = require('internal/os');
@@ -105,9 +105,9 @@ function tmpdir() {
105105
if (path.length > 1 && path.endsWith('\\') && !path.endsWith(':\\'))
106106
path = path.slice(0, -1);
107107
} else {
108-
path = process.env.TMPDIR ||
109-
process.env.TMP ||
110-
process.env.TEMP ||
108+
path = safeGetenv('TMPDIR') ||
109+
safeGetenv('TMP') ||
110+
safeGetenv('TEMP') ||
111111
'/tmp';
112112
if (path.length > 1 && path.endsWith('/'))
113113
path = path.slice(0, -1);

src/node_util.cc

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ using v8::Object;
1414
using v8::Private;
1515
using v8::Promise;
1616
using v8::Proxy;
17+
using v8::String;
1718
using v8::Value;
1819

1920

@@ -181,6 +182,16 @@ void PromiseReject(const FunctionCallbackInfo<Value>& args) {
181182
args.GetReturnValue().Set(ret.FromMaybe(false));
182183
}
183184

185+
void SafeGetenv(const FunctionCallbackInfo<Value>& args) {
186+
CHECK(args[0]->IsString());
187+
Utf8Value strenvtag(args.GetIsolate(), args[0]);
188+
std::string text;
189+
if (!node::SafeGetenv(*strenvtag, &text)) return;
190+
args.GetReturnValue()
191+
.Set(String::NewFromUtf8(
192+
args.GetIsolate(), text.c_str(),
193+
v8::NewStringType::kNormal).ToLocalChecked());
194+
}
184195

185196
void Initialize(Local<Object> target,
186197
Local<Value> unused,
@@ -232,6 +243,8 @@ void Initialize(Local<Object> target,
232243
env->SetMethod(target, "createPromise", CreatePromise);
233244
env->SetMethod(target, "promiseResolve", PromiseResolve);
234245
env->SetMethod(target, "promiseReject", PromiseReject);
246+
247+
env->SetMethod(target, "safeGetenv", SafeGetenv);
235248
}
236249

237250
} // namespace util

test/parallel/test-util-internal.js

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,17 @@ const assert = require('assert');
66
const fixtures = require('../common/fixtures');
77

88
const binding = process.binding('util');
9-
const kArrowMessagePrivateSymbolIndex = binding.arrow_message_private_symbol;
9+
const {
10+
arrow_message_private_symbol: kArrowMessagePrivateSymbolIndex,
11+
safeGetenv
12+
} = binding;
13+
14+
for (const oneEnv in process.env) {
15+
assert.strictEqual(
16+
safeGetenv(oneEnv),
17+
process.env[oneEnv]
18+
);
19+
}
1020

1121
function getHiddenValue(obj, index) {
1222
return function() {

0 commit comments

Comments
 (0)