Skip to content

Conversation

@huntr-helper
Copy link

https://huntr.dev/users/bbeale has fixed the Arbitrary Code Injection vulnerability 🔨. bbeale 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 | #7
Vulnerability README | https://github.com/418sec/huntr/blob/master/bounties/npm/mongo-parse/1/README.md

User Comments:

📊 Metadata *

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

⚙️ Description *

Sanitizing the user input strings and then building the function string using template literals instead of string concatenation.

💻 Technical Description *

User input is now being sanitized with special concern to potentially dangerous JavaScript functions. Additional safeguards include using one of the libraries mentioned in the comments -- which is eval-sanitizer. I specified the option to not allow function calls.

🐛 Proof of Concept (PoC) *

this.test('malicious string', function() {
    var query = "} + eval(\'console.log(\"my ver evil stuff here\");\')//";
    try{
        parser.parse(query);
    } catch (e) {
        this.ok(deepEqual(e, EvalError))
    }
})

🔥 Proof of Fix (PoF) * / 👍 User Acceptance Testing (UAT)

mongo-parse-fix

bbeale and others added 3 commits August 24, 2020 12:45
@fresheneesz
Copy link
Owner

Thanks for the pull request! So I don't want sanitization to be the default in mongo-parse. Most use cases of this library don't require executing external queries. However, I'm willing to accept this if it is changed to be optional as a parameter. An example API I would accept:

const mongoParse = require ('mongo-parse')

const parser = mongoParse.sanitized(sanitizationFunction)

let query = parser.parse(unsafeQuery)

// ...

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