Skip to content

Commit 38ab1e9

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 <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>
1 parent 8c9aaac commit 38ab1e9

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
@@ -915,8 +915,14 @@ int ProcessGlobalArgs(std::vector<std::string>* args,
915915

916916
if (!errors->empty()) return 9;
917917

918-
for (const std::string& cve : per_process::cli_options->security_reverts)
919-
Revert(cve.c_str());
918+
std::string revert_error;
919+
for (const std::string& cve : per_process::cli_options->security_reverts) {
920+
Revert(cve.c_str(), &revert_error);
921+
if (!revert_error.empty()) {
922+
errors->emplace_back(std::move(revert_error));
923+
return 12;
924+
}
925+
}
920926

921927
auto env_opts = per_process::cli_options->per_isolate->per_env;
922928
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)