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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Security Fix for Command Injection - huntr.dev #392

Merged
merged 3 commits into from
Oct 27, 2020
Merged

Conversation

huntr-helper
Copy link

@huntr-helper huntr-helper commented Jun 16, 2020

https://huntr.dev/app/users/toufik-airane has fixed the Command Injection vulnerability 🔨. toufik-airane 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
GitHub Issue URL | #391
Vulnerability README | https://github.com/418sec/huntr/blob/master/bounties/npm/jison/1/README.md

User Comments:

📊 Metadata

Please enter the direct URL for this bounty on huntr.dev. This is compulsory and will help us process your bounty submission quicker.

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

⚙️ Description

Describe your fix clearly and concisely - imagine you are describing it to a non-technical friend.

This patch fixes a command injection in Jison package.
A command injection is a cybersecurity vulnerability that could lead to execute arbitrary commands on the host operating system over a vulnerable application. (OWASP)

💻 Technical Description

Describe in-depth, the technical implementation of the proposed security fix. Imagine you are describing it to a NASA engineer.

The package handled untrusted user input inside an exec() function. exec() is used to execute operating system command with the privilege of the application. I've simply removed exec function for a more trusted one: execFile().

🐛 Proof of Concept (PoC)

Provide the vulnerability exploit to show the security issue you're fixing.

>  Jison [master] ⚡  node csharp.js ";touch HACKED"
Executing: jison ;touch HACKED

>  Jison [master] ⚡  ls
HACKED

🔥 Proof of Fix (PoF)

Replay the vulnerability exploit to show the successful fix and mitigation of the vulnerability.

>  Jison [master] ⚡  node csharp.js ";touch HACKED"
Executing: jison ;touch HACKED
Error: Command failed: jison ;touch HACKED

Error: ENOENT: no such file or directory, open ';touch HACKED'
    ...
  errno: -2,
  syscall: 'open',
  code: 'ENOENT',
  path: ';touch HACKED'
}

    ...
  killed: false,
  code: 1,
  signal: null,
  cmd: 'jison ;touch HACKED'
}

👍 User Acceptance Testing (UAT)

Run a unit test or a legitimate use case to prove that your fix does not introduce breaking changes.

 Jison [master] ⚡  yarn test
yarn run v1.22.4
$ node tests/all-tests.js
Running all tests: ...
Passed:91 Failed:0 Errors:2

The errors aren't coming from the change.

toufik-airane and others added 3 commits June 12, 2020 21:06
Fix minor issue with a sneaky space.
Fix OS Command Injection on Jison
@JamieSlome
Copy link
Contributor

@zaach - let me know if you have any questions or thoughts.

Cheers! 🍰

@zaach zaach merged commit 0c8a41b into zaach:master Oct 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants