Skip to content

Commit 9ac2eb8

Browse files
committed
Fix CWE-78 OS Command Injection in react-devtools editor.js
- Add SHELL_METACHARACTERS_RE to reject paths and editor binaries containing shell operators (&, |, ;, etc.) before they reach cmd.exe /C on Windows, preventing OS command injection. - Parse VISUAL/EDITOR env vars through shell-quote parse() consistent with how REACT_EDITOR is already handled, neutralising metacharacters in those values before they are used as the spawned editor binary. - Validate maybeRelativePath in getValidFilePath (line 116 entry point) so injected paths are rejected before reaching launchEditor. Resolves: CWE-78 (Improper Neutralization of Special Elements used in an OS Command)
1 parent f8b8507 commit 9ac2eb8

File tree

1 file changed

+24
-3
lines changed

1 file changed

+24
-3
lines changed

packages/react-devtools-core/src/editor.js

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@ import {basename, join, isAbsolute} from 'path';
1212
import {execSync, spawn} from 'child_process';
1313
import {parse} from 'shell-quote';
1414

15+
// Characters that cmd.exe interprets as shell operators, enabling OS command
16+
// injection (CWE-78) when user-controlled strings are passed via /C.
17+
const SHELL_METACHARACTERS_RE = /[;&|`$<>()\n\r]/;
18+
1519
function isTerminalEditor(editor: string): boolean {
1620
switch (editor) {
1721
case 'vim':
@@ -97,11 +101,13 @@ function guessEditor(): Array<string> {
97101
}
98102
}
99103

100-
// Last resort, use old-school env vars
104+
// Last resort, use old-school env vars.
105+
// Parse via shell-quote (same as REACT_EDITOR) so the value is split on
106+
// spaces but shell metacharacters are not expanded into a shell command.
101107
if (process.env.VISUAL) {
102-
return [process.env.VISUAL];
108+
return parse(process.env.VISUAL);
103109
} else if (process.env.EDITOR) {
104-
return [process.env.EDITOR];
110+
return parse(process.env.EDITOR);
105111
}
106112

107113
return [];
@@ -116,6 +122,14 @@ export function getValidFilePath(
116122
// We use relative paths at Facebook with deterministic builds.
117123
// This is why our internal tooling calls React DevTools with absoluteProjectRoots.
118124
// If the filename is absolute then we don't need to care about this.
125+
126+
// Reject paths containing shell metacharacters to prevent CWE-78 OS Command
127+
// Injection. On Windows, file paths are passed through cmd.exe which
128+
// interprets characters like &, |, ; as shell operators.
129+
if (SHELL_METACHARACTERS_RE.test(maybeRelativePath)) {
130+
return null;
131+
}
132+
119133
if (isAbsolute(maybeRelativePath)) {
120134
if (existsSync(maybeRelativePath)) {
121135
return maybeRelativePath;
@@ -161,6 +175,13 @@ export function launchEditor(
161175
return;
162176
}
163177

178+
// Reject editor binaries containing shell metacharacters to prevent CWE-78.
179+
// On Windows the editor string is passed to cmd.exe /C which would interpret
180+
// operators like & and | as command separators.
181+
if (SHELL_METACHARACTERS_RE.test(editor)) {
182+
return;
183+
}
184+
164185
let args = destructuredArgs;
165186

166187
if (lineNumber) {

0 commit comments

Comments
 (0)