Skip to content

Commit

Permalink
src: runtime deprecate process.umask()
Browse files Browse the repository at this point in the history
This commit runtime deprecates calling process.umask() with
no arguments.

PR-URL: #32499
Fixes: #32321
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
cjihrig committed Apr 1, 2020
1 parent 75ee5b2 commit 60c4c2b
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 4 deletions.
8 changes: 4 additions & 4 deletions doc/api/deprecations.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
<a id="DEP0XXX"></a>
### DEP0XXX: `process.umask()` with no arguments
<a id="DEP0139"></a>
### DEP0139: `process.umask()` with no arguments
<!-- YAML
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/32499
description: Documentation-only deprecation.
description: Runtime deprecation.
-->
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
Expand Down
8 changes: 8 additions & 0 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_;
}
Expand Down
3 changes: 3 additions & 0 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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<double> destroy_async_id_list_;

Expand Down
11 changes: 11 additions & 0 deletions src/node_process_methods.cc
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,17 @@ static void Umask(const FunctionCallbackInfo<Value>& 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<mode_t>(old));
} else {
Expand Down
7 changes: 7 additions & 0 deletions test/parallel/test-process-umask.js
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down

0 comments on commit 60c4c2b

Please sign in to comment.