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

[GHSA-4fr2-j4g9-mppf] Prototype Pollution in deephas #5039

Open
wants to merge 1 commit into
base: tariqhawis/advisory-improvement-5039
Choose a base branch
from

Conversation

tariqhawis
Copy link

Updates

  • Affected products
  • CVSS v3
  • Severity

Comments
The advisory claiming a prototype pollution vulnerability in deephas versions 1.0.0 through 1.0.5 is a false positive. While the link to the original advisory is broken, I’ve traced the data flow in the codebase and reviewed the maintainer’s test suite. Below is my analysis, supported by the provided proof-of-concept:

var dh = require("deephas"),
obj = {};
dh.set(obj,'__proto__.isAdmin',true);
console.log({}.isAdmin) // this suppose to return true.

Upon analyzing the relevant code block, the issue appears to be misunderstood:

        var items = str.split('.');
        var initial = items.slice(0, items.length - 1); // ['__proto__']
        var last = items.slice(items.length - 1); // polluted
        var test = initial.reduce(indexTrue, obj); // {} << fail reference to the prototype
        test[last] = val;  // {}.isAdmin = true <-- the property is created but not in the prototype 

The path proto.isAdmin is serialized into properties and processed as follows:
1. On Line 4, the reduce function fails to reference the prototype, meaning no link to Object.prototype is established.
2. As a result, the statement test[last] = val on Line 5 creates the property isAdmin on the object test, but only as a direct property. It does not modify the global prototype.

Since the prototype reference fails, no pollution occurs, and the proof-of-concept does not achieve the expected behavior.

Given this analysis, I kindly request that this advisory be removed from the database to prevent unnecessary confusion.

@github-actions github-actions bot changed the base branch from main to tariqhawis/advisory-improvement-5039 November 27, 2024 01:29
@shelbyc
Copy link
Contributor

shelbyc commented Nov 27, 2024

👋 Good morning @tariqhawis, I think this vulnerability is likely to be valid because the maintainer treated the report as valid and provided a fix in sharpred/deepHas@2fe0117. For now, I plan to leave the advisory in a reviewed state and not withdraw it.

Have you considered bringing your findings to Mend (formerly WhiteSource), the CVE Numbering Authority that issued CVE-2020-28271? WhiteSource advisories are now at mend.io, and the vendor advisory with proof of concept for CVE-2020-28271 is available at https://www.mend.io/vulnerability-database/CVE-2020-28271. If you wish to change a CVE record, the official way to request a change is to email the CNA at the email address listed on their CNA partner profile at https://www.cve.org/PartnerInformation/ListofPartners/partner/Mend.

Copy link

👋 This pull request has been marked as stale because it has been open with no activity. You can: comment on the issue or remove the stale label to hold stale off for a while, add the Keep label to hold stale off permanently, or do nothing. If you do nothing this pull request will be closed eventually by the stale bot. Please see CONTRIBUTING.md for more policy details.

@github-actions github-actions bot added the Stale label Dec 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants