From 60c4c2b6c557efbb2f8f3a3de147baf987931d41 Mon Sep 17 00:00:00 2001 From: cjihrig Date: Thu, 26 Mar 2020 00:55:39 -0400 Subject: [PATCH] src: runtime deprecate process.umask() This commit runtime deprecates calling process.umask() with no arguments. PR-URL: https://github.com/nodejs/node/pull/32499 Fixes: https://github.com/nodejs/node/issues/32321 Reviewed-By: Ben Noordhuis Reviewed-By: Anna Henningsen Reviewed-By: James M Snell --- doc/api/deprecations.md | 8 ++++---- src/env-inl.h | 8 ++++++++ src/env.h | 3 +++ src/node_process_methods.cc | 11 +++++++++++ test/parallel/test-process-umask.js | 7 +++++++ 5 files changed, 33 insertions(+), 4 deletions(-) diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index bb3224a7360686..4b2c7da0474a70 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -2635,16 +2635,16 @@ modules is unsupported. It is deprecated in favor of [`require.main`][], because it serves the same purpose and is only available on CommonJS environment. - -### DEP0XXX: `process.umask()` with no arguments + +### DEP0139: `process.umask()` with no arguments -Type: Documentation-only +Type: Runtime Calling `process.umask()` with no arguments causes the process-wide umask to be written twice. This introduces a race condition between threads, and is a diff --git a/src/env-inl.h b/src/env-inl.h index 9bf57686060b36..de2e68e11c8d08 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -910,6 +910,14 @@ void Environment::set_filehandle_close_warning(bool on) { emit_filehandle_warning_ = on; } +bool Environment::emit_insecure_umask_warning() const { + return emit_insecure_umask_warning_; +} + +void Environment::set_emit_insecure_umask_warning(bool on) { + emit_insecure_umask_warning_ = on; +} + inline uint64_t Environment::thread_id() const { return thread_id_; } diff --git a/src/env.h b/src/env.h index 719aca19a13803..82695aa392f0ba 100644 --- a/src/env.h +++ b/src/env.h @@ -1065,6 +1065,8 @@ class Environment : public MemoryRetainer { inline bool filehandle_close_warning() const; inline void set_filehandle_close_warning(bool on); + inline bool emit_insecure_umask_warning() const; + inline void set_emit_insecure_umask_warning(bool on); inline void ThrowError(const char* errmsg); inline void ThrowTypeError(const char* errmsg); @@ -1285,6 +1287,7 @@ class Environment : public MemoryRetainer { bool emit_env_nonstring_warning_ = true; bool emit_err_name_warning_ = true; bool emit_filehandle_warning_ = true; + bool emit_insecure_umask_warning_ = true; size_t async_callback_scope_depth_ = 0; std::vector destroy_async_id_list_; diff --git a/src/node_process_methods.cc b/src/node_process_methods.cc index 4d53749f0989f5..88f4c1cfbd0249 100644 --- a/src/node_process_methods.cc +++ b/src/node_process_methods.cc @@ -245,6 +245,17 @@ static void Umask(const FunctionCallbackInfo& args) { uint32_t old; if (args[0]->IsUndefined()) { + if (env->emit_insecure_umask_warning()) { + env->set_emit_insecure_umask_warning(false); + if (ProcessEmitDeprecationWarning( + env, + "Calling process.umask() with no arguments is prone to race " + "conditions and is a potential security vulnerability.", + "DEP0139").IsNothing()) { + return; + } + } + old = umask(0); umask(static_cast(old)); } else { diff --git a/test/parallel/test-process-umask.js b/test/parallel/test-process-umask.js index 85af91620709c7..e74c71e1935246 100644 --- a/test/parallel/test-process-umask.js +++ b/test/parallel/test-process-umask.js @@ -40,6 +40,13 @@ if (common.isWindows) { mask = '0664'; } +common.expectWarning( + 'DeprecationWarning', + 'Calling process.umask() with no arguments is prone to race conditions ' + + 'and is a potential security vulnerability.', + 'DEP0139' +); + const old = process.umask(mask); assert.strictEqual(process.umask(old), parseInt(mask, 8));