[BUG] Prototype Pollution vulnerability in @xmldom/xmldom@0.7.5

[ghost]

Hello,

Snyk identified a Prototype Pollution vulnerability in @xmldom/xmldom@0.7.5 which is a dependency of passport-saml@3.2.2.

https://security.snyk.io/vuln/SNYK-JS-XMLDOMXMLDOM-3042243

Please could you update @xmldom/xmldom to 0.8.3.

Thanks,

Ben

[jftanner]

It looks like it's not a direct dependency.

Rather, it's a direct dependency of node-saml at v0.8.3, as well as its nested dependencies xml-crypto and xml-encryption at v0.7.5.

Unfortunately, node-saml hasn't had an update in a year.

An issue is already open for xml-crypto (node-saml/xml-crypto#260), but not xml-encryption.

[jftanner]

Digging in, it looks like xml-crypto and xml-encryption packages only use the DOMParser constructor and instance parseFromString method.

I can't quite make sense of the code enough to confidently say if the vulnerable copy method is or is not used. So, for my purposes, I'm treating this vulnerability as "maybe applies". :(

I've submitted PR xmldom/xmldom#441 to hopefully backport the fix into 0.7.x, so that this can be mitigated without new versions of the intermediate dependencies. (Both xml-crypto and xml-encryption use ^0.7.0, so a new 0.7.x release should be picked up automatically with lockfile maintenance.)

[srd90]

It looks like it's not a direct dependency.

@jftanner I believe @BenBullock1992 was talking about 3.2.2 which lives in 3.x branch: https://github.com/node-saml/passport-saml/tree/3.x
Version 3.2.2's package.json has (8b6b2f28 == 3.2.2):

passport-saml/package.json

Line 53 in 8b6b2f2

"@xmldom/xmldom": "^0.7.5",

passport-saml was splitted to passport-saml and node-saml starting from 4.x.x.

Unfortunately, node-saml hasn't had an update in a year.

You might be referring to unscope node-saml https://www.npmjs.com/package/node-saml which is obsolete because at the moment passport-saml and node-saml current >= 4.x.x packages are released as @node-saml/passport-saml and @node-saml/node-saml. I.e.:

• https://www.npmjs.com/package/@node-saml/node-saml
  • see versions tab
• https://www.npmjs.com/package/@node-saml/passport-saml
  • see versions tab

About scoping change see additional information from #729

fwiw, If you were using unscope node-saml npmjs package https://www.npmjs.com/package/node-saml be aware that it had releases from two unrelated github repository (see node-saml/node-saml#21 )

[jftanner]

You're right @srd90. Helps if I read more thoroughly. Thanks for the correction.

Fortunately, the PR to backport the fix to @xmldom/xmldom 0.7.x will also apply to passport-saml, once released.

[jftanner]

xml-crypto v3.0.0 was released this morning, with an updated version of xmldom.

[cjbarth]

Unless there is a CVE published for a dependency that the 3.x series uses, we likely won't be updating it. Even then, we hope to have the 4.x series released in November, after which we won't update the 3.x series at all. For master, dependabot will help us with the dependencies and we try to stay up with that as you can tell from the commit history and changelog.

[chayanray]

CVE: https://nvd.nist.gov/vuln/detail/CVE-2022-37616
FYI: @cjbarth

[abcd-ca]

@cjbarth Can this issue be re-opened? There is a CVE☝️.

Update: I removed npm_modules and npm install and the critical vulnerability for xmldom went away. I guess they updated it in that project.

[cjbarth]

Feel free to make a PR against the 3.x branch. 4.0.1 is already released with the fix.