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

CVE-2022-37617/ Prototype pollution found in resolve-shims.js #245

Closed
secdevlpr26 opened this issue Oct 10, 2022 · 4 comments · Fixed by #246
Closed

CVE-2022-37617/ Prototype pollution found in resolve-shims.js #245

secdevlpr26 opened this issue Oct 10, 2022 · 4 comments · Fixed by #246

Comments

@secdevlpr26
Copy link

secdevlpr26 commented Oct 10, 2022

Prototype pollution vulnerability in function resolveShims in resolve-shims.js in thlorenz browserify-shim 3.8.15 via the k variable in resolve-shims.js.
The prototype pollution vulnerability can be mitigated with several best practices described here: https://learn.snyk.io/lessons/prototype-pollution/javascript/

@bendrucker
Copy link
Collaborator

@bendrucker
Copy link
Collaborator

You can definitely overwrite the prototype for exposeGlobals:

exposeGlobals[k] = exp;

But I'm not seeing a way that you could pollute the global scope, e.g., Object.prototype. Prototype pollution typically involves some form of recursion or at least two levels of property access so that you can write to the objects within a target object. Here there's only one level of property access, exposeGlobals[k].

Beyond that, I'm not even sure overwriting exposeGlobals is easily exploitable. Before it's returned, it gets passed to Object.keys, which will only return own properties:

function mapifyExposeGlobals(exposeGlobals) {
return Object.keys(exposeGlobals)

No methods are called so I'm not seeing a way to achieve code execution.

Would appreciate some clarification on whether there's a substantive vulnerability here or whether this was potentially vulnerable code identified through automated static analysis.

In any case, guarding against prototype assignment is easy enough.

@secdevlpr26
Copy link
Author

Hello,

The code has been flagged as a potentially vulnerable code and the CVE has the sink and the path details of the code.

All the reports are based on the research work of my colleague (you can find her paper's link below) and I am reporting them here as per her analysis and records.

https://dl.acm.org/doi/pdf/10.1145/3488932.3497769 - This is the published paper with the Github link to her static analysis tool.
Thanks

@thlorenz
Copy link
Owner

This whole package depends on adding a property to the global since that is what certain packages expect.
Thus it is only usd in scenarios where other packages already overwrite globals the exact same way.
Additionally most of the code you keep flagging runs as part of the build step (not when the web app loads in the browser), but I'm not sure that your tools are aware of that.

In more modern scenarios where this is not desired/necessary browserify-shim is not used.

Please stop reporting these issues of the same family here. It is noise and adds overhead to the maintainers work.

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 a pull request may close this issue.

3 participants