src: pass along errors from --security-reverts

addaleax · addaleax · commit 38ab1e9ade1e · 2019-01-22T22:53:26.000+01:00
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: #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>
diff --git 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
@@ -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
@@ -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}`);
