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-37609]/Prototype pollution found in options.js. #2106

Closed
secdevlpr26 opened this issue Oct 10, 2022 · 5 comments
Closed

[CVE-2022-37609]/Prototype pollution found in options.js. #2106

secdevlpr26 opened this issue Oct 10, 2022 · 5 comments

Comments

@secdevlpr26
Copy link

Prototype pollution vulnerability in beautify-web js-beautify 1.13.7 via the name variable in options.js.
The prototype pollution vulnerability can be mitigated with several best practices described here: https://learn.snyk.io/lessons/prototype-pollution/javascript/

@bitwiseman
Copy link
Member

bitwiseman commented Oct 21, 2022

This report is incomplete to the point of useless. The version in the report as over a year out of date. Please reopen with specific line numbers or other details.

@zjhua2002
Copy link

@bitwiseman please kindly analyze CVE-2022-37609 which was reported recently with high Base Score: [9.8 CRITICAL] and provide assessments, thanks.

@jackieju
Copy link

jackieju commented Oct 28, 2022

The suspicious code identified as weakness is here, according to cve record:
https://github.com/beautify-web/js-beautify/blob/6fa891e982cc3d615eed9a1a20a4fc50721bff16/js/src/core/options.js#L167
Actually the suspicious code will not lead to object prototype pollution, because it's not recursive or deep copy of two object.
Only
a["_proto_"][x]=1
will pollute.
But
a["__proto__"]={x:1}
will not pollute.
Here is the test, which is simple:
=== test1 ====

> a={}
{}
> a.__proto__.c=1
1
> b={}
{}
> b.c
1
==== test2 ====
> a2={}
{}
> a2.__proto__={c:1}
{ c: 1 }
>  b2={}
{}
> b2.c 
undefined

Above testing is passed in node.js and browser(chrome)

A merge function that can leads to prototype pollution must be recursive or cop depth >1.
e.g.

function merge(t,s){
    for (let k in s){
        if ( k in s && k in t)
       {             // here do recursive copy            
           merge(t[k], s[k]);        
       }
       else
      {             
            t[k]=s[k];        
      }
    }
}

But that line of js_beautify code only do 1 level copy.
I modify js_beautify code to test, add code like below:

var finalOpts = {};
 var a={};
 allOptions=JSON.parse('{"js":{"{}proto{}":{"role":"admin"}}, "aa":1, "{}proto{}":{"role":"admin"}}');
 
 allOptions = _normalizeOpts(allOptions);
 var name;
 for (name in allOptions) {
   if (name !== childFieldName)
{
     finalOpts[name] = allOptions[name];  // the suspicious line
 }
 }
   console.log("===>aa.role:"+a.role);
   
 //merge in the per type settings for the childFieldName
 if (childFieldName && allOptions[childFieldName]) {
   for (name in allOptions[childFieldName])
{       finalOpts[name] = allOptions[childFieldName][name];     }
 }
 console.log("===>a.role:"+aa.role);

In node.js and browser(Chrome), it cannot make the pollution happen.
So the suspicious code is safe. js-beautify should be innocent.

@meena-kaliswamy
Copy link

meena-kaliswamy commented Nov 2, 2022

@bitwiseman I see that the tag (status: needs more info) has been removed, but the code has not been updated with @jackieju 's fix. I'm kinda confused - was a fix provided, or is it saying that the sus code is in fact not sus?

@bitwiseman
Copy link
Member

@meena-kaliswamy
If you read his commens, it says:

Actually the suspicious code will not lead to object prototype pollution, because it's not recursive or deep copy of two object.

And since js-beautify doesn't do that, no update is needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants