Skip to content

Commit d9a8113

Browse files
committed
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: #25466 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent 8cc936a commit d9a8113

File tree

3 files changed

+26
-5
lines changed

3 files changed

+26
-5
lines changed

src/node.cc

+8-2
Original file line numberDiff line numberDiff line change
@@ -896,8 +896,14 @@ int ProcessGlobalArgs(std::vector<std::string>* args,
896896

897897
if (!errors->empty()) return 9;
898898

899-
for (const std::string& cve : per_process::cli_options->security_reverts)
900-
Revert(cve.c_str());
899+
std::string revert_error;
900+
for (const std::string& cve : per_process::cli_options->security_reverts) {
901+
Revert(cve.c_str(), &revert_error);
902+
if (!revert_error.empty()) {
903+
errors->emplace_back(std::move(revert_error));
904+
return 12;
905+
}
906+
}
901907

902908
auto env_opts = per_process::cli_options->per_isolate->per_env;
903909
if (std::find(v8_args.begin(), v8_args.end(),

src/node_revert.h

+4-3
Original file line numberDiff line numberDiff line change
@@ -43,13 +43,14 @@ inline void Revert(const reversion cve) {
4343
printf("SECURITY WARNING: Reverting %s\n", RevertMessage(cve));
4444
}
4545

46-
inline void Revert(const char* cve) {
46+
inline void Revert(const char* cve, std::string* error) {
4747
#define V(code, label, _) \
4848
if (strcmp(cve, label) == 0) return Revert(SECURITY_REVERT_##code);
4949
SECURITY_REVERSIONS(V)
5050
#undef V
51-
printf("Error: Attempt to revert an unknown CVE [%s]\n", cve);
52-
exit(12);
51+
*error = "Error: Attempt to revert an unknown CVE [";
52+
*error += cve;
53+
*error += ']';
5354
}
5455

5556
inline bool IsReverted(const reversion cve) {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
'use strict';
2+
require('../common');
3+
const assert = require('assert');
4+
const { spawnSync } = require('child_process');
5+
const os = require('os');
6+
7+
const { signal, status, output } =
8+
spawnSync(process.execPath, ['--security-reverts=not-a-cve']);
9+
assert.strictEqual(signal, null);
10+
assert.strictEqual(status, 12);
11+
assert.strictEqual(
12+
output[2].toString(),
13+
`${process.execPath}: Error: ` +
14+
`Attempt to revert an unknown CVE [not-a-cve]${os.EOL}`);

0 commit comments

Comments
 (0)