diff options
author | Anna Henningsen <anna@addaleax.net> | 2019-01-12 20:55:58 +0100 |
---|---|---|
committer | Anna Henningsen <anna@addaleax.net> | 2019-01-22 22:53:26 +0100 |
commit | 38ab1e9ade1e473f1c0b7a1e7535d41654506a3d (patch) | |
tree | 9c1843e7472f9c2654c2be0755256267245ec5a9 | |
parent | 8c9aaacb333a0223243eda9b3551dbf26fdb260a (diff) | |
download | android-node-v8-38ab1e9ade1e473f1c0b7a1e7535d41654506a3d.tar.gz android-node-v8-38ab1e9ade1e473f1c0b7a1e7535d41654506a3d.tar.bz2 android-node-v8-38ab1e9ade1e473f1c0b7a1e7535d41654506a3d.zip |
src: pass along errors from `--security-reverts`
Pass along errors from `Revert()` when a security revert
is unknown (which currently applies to all possible values).
Previously, we would unconditionally call `exit()`, which is
not nice for embedding use cases, and could crash because we
were holding a lock for a mutex in `ProcessGlobalArgs()` that
would be destroyed by calling `exit()`.
Also, add a regression test that makes sure that the process
exits with the right exit code and not a crash.
PR-URL: https://github.com/nodejs/node/pull/25466
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
-rw-r--r-- | src/node.cc | 10 | ||||
-rw-r--r-- | src/node_revert.h | 7 | ||||
-rw-r--r-- | test/parallel/test-security-revert-unknown.js | 14 |
3 files changed, 26 insertions, 5 deletions
diff --git a/src/node.cc b/src/node.cc index 2766654e9a..7e0fb828e5 100644 --- a/src/node.cc +++ b/src/node.cc @@ -915,8 +915,14 @@ int ProcessGlobalArgs(std::vector<std::string>* args, if (!errors->empty()) return 9; - for (const std::string& cve : per_process::cli_options->security_reverts) - Revert(cve.c_str()); + std::string revert_error; + for (const std::string& cve : per_process::cli_options->security_reverts) { + Revert(cve.c_str(), &revert_error); + if (!revert_error.empty()) { + errors->emplace_back(std::move(revert_error)); + return 12; + } + } auto env_opts = per_process::cli_options->per_isolate->per_env; if (std::find(v8_args.begin(), v8_args.end(), diff --git a/src/node_revert.h b/src/node_revert.h index 4c0ebcd9fd..38e2ba7105 100644 --- a/src/node_revert.h +++ b/src/node_revert.h @@ -43,13 +43,14 @@ inline void Revert(const reversion cve) { printf("SECURITY WARNING: Reverting %s\n", RevertMessage(cve)); } -inline void Revert(const char* cve) { +inline void Revert(const char* cve, std::string* error) { #define V(code, label, _) \ if (strcmp(cve, label) == 0) return Revert(SECURITY_REVERT_##code); SECURITY_REVERSIONS(V) #undef V - printf("Error: Attempt to revert an unknown CVE [%s]\n", cve); - exit(12); + *error = "Error: Attempt to revert an unknown CVE ["; + *error += cve; + *error += ']'; } inline bool IsReverted(const reversion cve) { diff --git a/test/parallel/test-security-revert-unknown.js b/test/parallel/test-security-revert-unknown.js new file mode 100644 index 0000000000..688076ce94 --- /dev/null +++ b/test/parallel/test-security-revert-unknown.js @@ -0,0 +1,14 @@ +'use strict'; +require('../common'); +const assert = require('assert'); +const { spawnSync } = require('child_process'); +const os = require('os'); + +const { signal, status, output } = + spawnSync(process.execPath, ['--security-reverts=not-a-cve']); +assert.strictEqual(signal, null); +assert.strictEqual(status, 12); +assert.strictEqual( + output[2].toString(), + `${process.execPath}: Error: ` + + `Attempt to revert an unknown CVE [not-a-cve]${os.EOL}`); |