From d9a8113a5b718e881b89867854521f4cb6b40cdb Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Sat, 12 Jan 2019 20:55:58 +0100
Subject: [PATCH] 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>
---
 src/node.cc                                   | 10 ++++++++--
 src/node_revert.h                             |  7 ++++---
 test/parallel/test-security-revert-unknown.js | 14 ++++++++++++++
 3 files changed, 26 insertions(+), 5 deletions(-)
 create mode 100644 test/parallel/test-security-revert-unknown.js

diff --git a/src/node.cc b/src/node.cc
index b3e94f51cec29f..ff9ac8494fbb83 100644
--- a/src/node.cc
+++ b/src/node.cc
@@ -896,8 +896,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 4c0ebcd9fd66b0..38e2ba71053691 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 00000000000000..688076ce94582a
--- /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}`);