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

Fix Arbitary Code Execution - #1

Merged
JamieSlome merged 1 commit into
418sec:masterfrom
alromh87:master
Sep 14, 2020
Merged

Fix Arbitary Code Execution#1
JamieSlome merged 1 commit into
418sec:masterfrom
alromh87:master

Conversation

@alromh87

@alromh87alromh87 commented Sep 9, 2020

Copy link
Copy Markdown

📊 Metadata *

Bounty URL: https://www.huntr.dev/bounties/1-npm-git-static

⚙️ Description *

git-static was vulnerable against arbitrary command injection because some user supplied inputs were taken and formatted inside the exec() function without prior validation.
After update Arbitary Code Execution is avoided by using execFile instead of exec

💻 Technical Description *

Arbitary Code Execution is avoided by using execFile() instead of exec() and passing arguments via parameters instead of composing strings, piping to grep was replaced with regexp

🐛 Proof of Concept (PoC) *

Install the package and run the below code:

constgitstatic=require("./");gitstatic.getSha('./'," '; whoami>HACKED #",(e,resp)=>{if(e)console.log(e)elseconsole.log(resp)});

It will create a file named HACKED in the working directory.

git-staticPOC

🔥 Proof of Fix (PoF) *

After fix no file is created

git-staticPOF

👍 User Acceptance Testing (UAT)

Commands can be executed normally

git-staticOK

Correct functionality after fix can be tested with this script:

constgitstatic=require("./");consthandleResp=(e,resp)=>{if(e)console.log(e)elseconsole.log(resp)};gitstatic.getBranches('./',handleResp);gitstatic.getSha('./','master',handleResp);gitstatic.getBranchCommits('./',handleResp);gitstatic.getCommit('./','',handleResp);gitstatic.getCommit('./','ccde678213daa6913d63b30e0f30113e0a6b0c7c',handleResp);gitstatic.getRelatedCommits('./','master','d6c64168559aacd41f8593e53b1e17159560228b',handleResp);gitstatic.getRelatedCommits('./','master','ccde678213daa6913d63b30e0f30113e0a6b0c7c',handleResp);gitstatic.getRelatedCommits('./','master','7101f0f77c9a174c2b0b0916d15dce273a2cab3f',handleResp);gitstatic.listCommits('./','ccde678213daa6913d63b30e0f30113e0a6b0c7c','d6c64168559aacd41f8593e53b1e17159560228b',handleResp);gitstatic.listAllCommits('./',handleResp);gitstatic.listTree('./','ccde678213daa6913d63b30e0f30113e0a6b0c7c',handleResp);

gitStaticOK2

@toufik-airanetoufik-airane left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM ☀️

@mufeedvhmufeedvh left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good fix! 👏🎉

LGTM

@bbealebbeale left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solid fix 👍 🎉

@Mik317Mik317 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 😄 🍰

Cheers,
Mik

@JamieSlome
JamieSlome merged commit 7778d0e into 418sec:masterSep 14, 2020
@huntr-helper

Copy link
Copy Markdown

Congratulations alromh87 - your fix has been selected! 🎉

Thanks for being part of the community & helping secure the world's open source code.
If you have any questions, please respond in the comments section, or hit us up on Discord. Your bounty is on its way - keep hunting!

Come join us on Discord

Sign up for freeto 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.

7 participants

@alromh87@huntr-helper@toufik-airane@bbeale@mufeedvh@Mik317@JamieSlome