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

Fixed insufficient input validation #2

Merged
merged 5 commits into from
Jun 29, 2020
Merged

Conversation

bbeale
Copy link

@bbeale bbeale commented Jun 20, 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-bson-objectid

⚙️ Description *

Added methods for creating new ObjectID from JSON after removing unwanted properties from it.

💻 Technical Description *

  • Checks that were previously at the beginning of the constructor moved into ObjectID.hasRequiredProps.
  • ObjectID.sanitizeObject then removes unwanted properties from the JSON.
  • ObjectID.createFromObject then creates an ObjectID from the scrubbed JSON.

🐛 Proof of Concept (PoC) *

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

🔥 Proof of Fix (PoF) *

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

👍 User Acceptance Testing (UAT)

Run the following unit test

it('should not allow insertion of an arbitrary property', function() {
    var json = {
      "mal_formkey": {
        "payload": "xxxx"
      },
      "_bsontype": "ObjectID",
      "id": "5eecccdc951ca34d04e3ff65",
    };

    var obj = ObjectID(json);
    obj.should.be.instanceof(ObjectID);
    obj.toString().should.eql("5eecccdc951ca34d04e3ff65");
  });

Copy link

@toufik-airane toufik-airane left a comment

Choose a reason for hiding this comment

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

Dear @bbeale,

Nice work.

Can you reset your commit on package-lock.json and package.json?

huntr sheriff

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.

Awesome fix @bbeale! 👏

Complete the changes requested by @toufik-airane and it's perfect! 🎉

huntr sheriff

@bbeale
Copy link
Author

bbeale commented Jun 23, 2020

Sorry about that one @toufik-airane @mufeedvh! Changes to package.json and package-lock.json have been reset.

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.

LGTM

Copy link

@toufik-airane toufik-airane left a comment

Choose a reason for hiding this comment

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

Awesome!

huntr

@JamieSlome JamieSlome merged commit d24f002 into 418sec:master Jun 29, 2020
@huntr-helper
Copy link
Member

Congratulations bbeale - 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!

@niftylettuce
Copy link

Hey folks - I'm going to manually cherry-pick this, but I'd love your support by having pull requests in future go upstream!

@niftylettuce
Copy link

Actually it appears there was a PR, but it had conflicts.

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

6 participants