Commit d9a8113
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 8cc936a commit d9a8113
3 files changed
Lines changed: 26 additions & 5 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -896,8 +896,14 @@ int ProcessGlobalArgs(std::vector<std::string>* args, | |||
| 896 | 896 | ||
| 897 | 897 | if (!errors->empty()) return 9; | |
| 898 | 898 | ||
| 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 | + } | ||
| 901 | 907 | ||
| 902 | 908 | auto env_opts = per_process::cli_options->per_isolate->per_env; | |
| 903 | 909 | if (std::find(v8_args.begin(), v8_args.end(), | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -43,13 +43,14 @@ inline void Revert(const reversion cve) { | |||
| 43 | 43 | printf("SECURITY WARNING: Reverting %s\n", RevertMessage(cve)); | |
| 44 | 44 | } | |
| 45 | 45 | ||
| 46 | - inline void Revert(const char* cve) { | ||
| 46 | + inline void Revert(const char* cve, std::string* error) { | ||
| 47 | 47 | #define V(code, label, _) \ | |
| 48 | 48 | if (strcmp(cve, label) == 0) return Revert(SECURITY_REVERT_##code); | |
| 49 | 49 | SECURITY_REVERSIONS(V) | |
| 50 | 50 | #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 += ']'; | ||
| 53 | 54 | } | |
| 54 | 55 | ||
| 55 | 56 | inline bool IsReverted(const reversion cve) { | |
| Original file line number | Diff line number | Diff 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