From 916cfeca774e83925466f9a171f11c9bc73e4756 Mon Sep 17 00:00:00 2001
From: "Jose M. Palacios Diaz" <jmpd1988@gmail.com>
Date: Thu, 1 Feb 2018 11:13:35 -0500
Subject: [PATCH] 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: https://github.com/nodejs/node/pull/18511
Fixes: https://github.com/nodejs/node/issues/9160
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
---
 lib/module.js                       |  7 +++++--
 lib/os.js                           |  8 ++++----
 src/node_util.cc                    | 13 +++++++++++++
 test/parallel/test-util-internal.js | 10 +++++++++-
 4 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/lib/module.js b/lib/module.js
index 77ed83423b9..42da5c01ce8 100644
--- a/lib/module.js
+++ b/lib/module.js
@@ -34,6 +34,7 @@ const {
   internalModuleReadJSON,
   internalModuleStat
 } = process.binding('fs');
+const { safeGetenv } = process.binding('util');
 const internalModule = require('internal/module');
 const preserveSymlinks = !!process.binding('config').preserveSymlinks;
 const experimentalModules = !!process.binding('config').experimentalModules;
@@ -697,10 +698,13 @@ Module._initPaths = function() {
   const isWindows = process.platform === 'win32';
 
   var homeDir;
+  var nodePath;
   if (isWindows) {
     homeDir = process.env.USERPROFILE;
+    nodePath = process.env.NODE_PATH;
   } else {
-    homeDir = process.env.HOME;
+    homeDir = safeGetenv('HOME');
+    nodePath = safeGetenv('NODE_PATH');
   }
 
   // $PREFIX/lib/node, where $PREFIX is the root of the Node.js installation.
@@ -719,7 +723,6 @@ Module._initPaths = function() {
     paths.unshift(path.resolve(homeDir, '.node_modules'));
   }
 
-  var nodePath = process.env.NODE_PATH;
   if (nodePath) {
     paths = nodePath.split(path.delimiter).filter(function(path) {
       return !!path;
diff --git a/lib/os.js b/lib/os.js
index 7c07a5b0d38..eb13139dba9 100644
--- a/lib/os.js
+++ b/lib/os.js
@@ -21,7 +21,7 @@
 
 'use strict';
 
-const { pushValToArrayMax } = process.binding('util');
+const { pushValToArrayMax, safeGetenv } = process.binding('util');
 const constants = process.binding('constants').os;
 const { deprecate } = require('internal/util');
 const { getCIDRSuffix } = require('internal/os');
@@ -127,9 +127,9 @@ function tmpdir() {
     if (path.length > 1 && path.endsWith('\\') && !path.endsWith(':\\'))
       path = path.slice(0, -1);
   } else {
-    path = process.env.TMPDIR ||
-           process.env.TMP ||
-           process.env.TEMP ||
+    path = safeGetenv('TMPDIR') ||
+           safeGetenv('TMP') ||
+           safeGetenv('TEMP') ||
            '/tmp';
     if (path.length > 1 && path.endsWith('/'))
       path = path.slice(0, -1);
diff --git a/src/node_util.cc b/src/node_util.cc
index 0c4eaa4aa73..1542b533f34 100644
--- a/src/node_util.cc
+++ b/src/node_util.cc
@@ -14,6 +14,7 @@ using v8::Object;
 using v8::Private;
 using v8::Promise;
 using v8::Proxy;
+using v8::String;
 using v8::Value;
 
 
@@ -174,6 +175,16 @@ void PromiseReject(const FunctionCallbackInfo<Value>& args) {
   args.GetReturnValue().Set(ret.FromMaybe(false));
 }
 
+void SafeGetenv(const FunctionCallbackInfo<Value>& args) {
+  CHECK(args[0]->IsString());
+  Utf8Value strenvtag(args.GetIsolate(), args[0]);
+  std::string text;
+  if (!node::SafeGetenv(*strenvtag, &text)) return;
+  args.GetReturnValue()
+      .Set(String::NewFromUtf8(
+            args.GetIsolate(), text.c_str(),
+            v8::NewStringType::kNormal).ToLocalChecked());
+}
 
 void Initialize(Local<Object> target,
                 Local<Value> unused,
@@ -225,6 +236,8 @@ void Initialize(Local<Object> target,
   env->SetMethod(target, "createPromise", CreatePromise);
   env->SetMethod(target, "promiseResolve", PromiseResolve);
   env->SetMethod(target, "promiseReject", PromiseReject);
+
+  env->SetMethod(target, "safeGetenv", SafeGetenv);
 }
 
 }  // namespace util
diff --git a/test/parallel/test-util-internal.js b/test/parallel/test-util-internal.js
index 5fda104baf1..ac7cf122294 100644
--- a/test/parallel/test-util-internal.js
+++ b/test/parallel/test-util-internal.js
@@ -8,9 +8,17 @@ const fixtures = require('../common/fixtures');
 const {
   getHiddenValue,
   setHiddenValue,
-  arrow_message_private_symbol: kArrowMessagePrivateSymbolIndex
+  arrow_message_private_symbol: kArrowMessagePrivateSymbolIndex,
+  safeGetenv
 } = process.binding('util');
 
+for (const oneEnv in process.env) {
+  assert.strictEqual(
+    safeGetenv(oneEnv),
+    process.env[oneEnv]
+  );
+}
+
 assert.strictEqual(
   getHiddenValue({}, kArrowMessagePrivateSymbolIndex),
   undefined);