Security Fix for Command Injection

[th13vn]

Q | A
Version Affected | *
Bug Fix | YES

Description Pull Request

Used child_process.execFileSync() instead of child_process.execSync().

Description Vulnerability

The use of the child_process function execSync() is highly discouraged if you accept user input and don't sanitize/escape them.
In the program, url param was passed into function openBrowser() will go to startBrowserProcess() and be used by execFileSync() (L92-L102). url was encoded by encodeURI(), but encodeURI() is not encoded some special characters ;,/?:@&=+$#-_.!~*'() so attacker could put $(command) into URL string and arbitrarily execute command. In addition, the $IFS could bypass white space encoded by encodeURI().

PoC

Create a .js file with the content below and run it, then the file /tmp/th13ntc can be illegally created.

// poc.js
var openBrowser = require('react-dev-utils/openBrowser');

openBrowser('http://example.com/#$(touch$IFS/tmp/th13ntc)');

Proof of Fix (PoF)

Use:

//code fixed
execFileSync(
  "osascript",
  ["openChrome.applescript", encodeURI(url), chromiumBrowser],
  {
    cwd: __dirname,
    stdio: "ignore",
  }
);

Replace:

execSync(
  'osascript openChrome.applescript "' +
    encodeURI(url) +
    '" "' +
    chromiumBrowser +
    '"',
  {
    cwd: __dirname,
    stdio: "ignore",
  }
);

User Acceptance Testing (UAT)

var openBrowser = require('react-dev-utils/openBrowser');
openBrowser('http://example.com/'); //works correctly

References

1. Credit to thientc from VNG Cloud Security Team with CodeQL Agent supported.
2. Past context: Security Fix for Command Injection - huntr.dev #10644

Occurrences

#L92-L102