Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Security Fix for RCE on "gitlogplus" - huntr.dev #59

Merged
merged 2 commits into from Sep 14, 2020

Conversation

huntr-helper
Copy link

https://huntr.dev/users/Asjidkalam has fixed the RCE on "gitlogplus" vulnerability 馃敤. Asjidkalam has been awarded $25 for fixing the vulnerability through the huntr bug bounty program 馃挼. Think you could fix a vulnerability like this?

Get involved at https://huntr.dev/

Q | A
Version Affected | ALL
Bug Fix | YES
Original Pull Request | 418sec#1
Vulnerability README | https://github.com/418sec/huntr/blob/master/bounties/npm/gitlogplus/1/README.md

User Comments:

馃搳 Metadata *

Code execution vulnerability

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

鈿欙笍 Description *

The gitlogplus module is vulnerable against an arbitrary command injection issue which is made possible since some user-inputs are executed inside a command which doesn't have validations of any kind. The argument options can be controlled by users without any sanitization. It was using exec() & execSync() function which is vulnerable to Command Injection if it accepts user input and it goes through any sanitization or escaping.

馃捇 Technical Description *

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

馃悰 Proof of Concept (PoC) *

Install the package and run the below code:

var git = require('gitlogplus');
git({repo:'.', number:'eeee; touch HACKED; #'})

A file named HACKED will be created in the current working directory.

image

馃敟 Proof of Fix (PoF) *

After applying the fix, run the PoC again and no files will be created. Hence command injection is mitigated.

image

馃憤 User Acceptance Testing (UAT)

Only execFile and execFileSync is used, no breaking changes introduced.

@hipstersmoothie hipstersmoothie added the patch Increment the patch version when merged label Sep 14, 2020
@hipstersmoothie hipstersmoothie merged commit 7ab4f61 into domharrington:master Sep 14, 2020
@hipstersmoothie
Copy link
Collaborator

馃殌 PR was released in v4.0.1 馃殌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch Increment the patch version when merged released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants