From 8f6c5870cc5fb9bad01f2a0c31bdb7a81b2ea571 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Sat, 11 Oct 2014 21:48:25 +0200 Subject: [PATCH] src: ignore risky env vars when setuid root On POSIX platforms, check that the uid and gid match the euid and egid respectively before looking up the environment variable. Before this commit, an i18n-enabled suid node would cheerfully load attacker-controlled ICU data through the NODE_ICU_DATA environment variable. This commit is not a complete fix. For example, it's up for debate what to do with the NODE_CHANNEL_FD environment variable. PR-URL: https://github.com/node-forward/node/pull/18 Reviewed-By: Fedor Indutny --- src/node.cc | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/node.cc b/src/node.cc index 69fbf47edc..ea41e36632 100644 --- a/src/node.cc +++ b/src/node.cc @@ -835,6 +835,16 @@ Local UVException(Isolate* isolate, } +// Look up environment variable unless running as setuid root. +inline const char* secure_getenv(const char* key) { +#ifndef _WIN32 + if (getuid() != geteuid() || getgid() != getegid()) + return NULL; +#endif + return getenv(key); +} + + #ifdef _WIN32 // Does about the same as strerror(), // but supports all windows error messages @@ -3424,7 +3434,7 @@ void Init(int* argc, #if defined(NODE_HAVE_I18N_SUPPORT) if (icu_data_dir == NULL) { // if the parameter isn't given, use the env variable. - icu_data_dir = getenv("NODE_ICU_DATA"); + icu_data_dir = secure_getenv("NODE_ICU_DATA"); } // Initialize ICU. // If icu_data_dir is NULL here, it will load the 'minimal' data. @@ -3664,7 +3674,7 @@ Environment* CreateEnvironment(Isolate* isolate, int Start(int argc, char** argv) { - const char* replaceInvalid = getenv("NODE_INVALID_UTF8"); + const char* replaceInvalid = secure_getenv("NODE_INVALID_UTF8"); if (replaceInvalid == NULL) WRITE_UTF8_FLAGS |= String::REPLACE_INVALID_UTF8;