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

Fix OS Command Injection on Jison #1

Merged
merged 2 commits into from
Jun 16, 2020
Merged

Fix OS Command Injection on Jison #1

merged 2 commits into from
Jun 16, 2020

Conversation

toufik-airane
Copy link

@toufik-airane toufik-airane commented Jun 12, 2020

πŸ“Š 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/app/bounties/open/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.

Fix minor issue with a sneaky space.
Copy link

@mufeedvh mufeedvh left a comment

Choose a reason for hiding this comment

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

Nice Work @toufik-airane! πŸŽ‰πŸ‘

LGTM

@JamieSlome JamieSlome merged commit 979a6ef into 418sec:master Jun 16, 2020
@huntr-helper
Copy link
Member

Congratulations toufik-airane - 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. Your bounty is on its way - keep hunting!

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