Skip to content
This repository has been archived by the owner on Dec 19, 2023. It is now read-only.

Command Injection vul fix: Replace execSync with execFileSync #2

Merged
merged 1 commit into from
Mar 3, 2021

Conversation

zpbrent
Copy link

@zpbrent zpbrent commented Mar 3, 2021

📊 Metadata *

react-dev-utils includes some utilities used by Create React App.

The function getProcessForPort in react-dev-utils is vulnerable to command injection.

Bounty URL: https://www.huntr.dev/bounties/1-npm-react-dev-utils/

⚙️ Description *

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

💻 Technical Description *

The use of the child_process function execSync() is highly discouraged if you accept user input and don't sanitize/escape them. This PR replaces it with execFileSync() which mitigates any possible Command Injections as it accepts input as arrays.

🐛 Proof of Concept (PoC) *

Create a .js file with the content below and run it, then the file pzhou@shu can be illegally created.

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

getProcessForPort('11;$(touch pzhou@shu)');

🔥 Proof of Fix (PoF) *

use "return execFileSync('lsof', ['-i:'+port, '-P', '-t', '-sTCP:LISTEN'], execOptions)" to replace "return execSync('lsof -i:' + port + ' -P -t -sTCP:LISTEN', execOptions)"

👍 User Acceptance Testing (UAT)

var getProcessForPort = require('react-dev-utils/getProcessForPort');

getProcessForPort(3000) // works correctly

🔗 Relates to...

418sec/huntr#1962

@huntr-helper
Copy link
Member

👋 Hello, @gaearon - @zpbrent has opened a PR to us with a fix for a potential vulnerability in your repository. To view the vulnerability, please refer to the bounty URL in the first comment, above.

Ultimately, you get to decide if the fix is 👍 or 👎. If you are happy with the fix, please write a new comment (@huntr-helper - LGTM) and we will open a PR to your repository with the fix. All remaining PRs for this vulnerability will be automatically closed.

If you have any questions or need support, come and join us on our community Discord!

@gaearon & @zpbrent - thank you for your efforts in securing the world’s open source code! 🎉


🔨 Want more security researchers protecting your repository?

Stick our badge on your README.md, and let security researchers know that they can win bounties protecting your repositories. Sounds cool, huh? 😎

Copy this small code snippet and insert it into your README.md:

[![huntr](https://cdn.huntr.dev/huntr_security_badge.svg)](https://huntr.dev)

👇 👇 👇

huntr

huntr-helper pushed a commit to 418sec/huntr that referenced this pull request Mar 3, 2021
@gaearon
Copy link

gaearon commented Mar 3, 2021

@huntr-helper - LGTM

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants