Prototype Pollution using .parse()

[keerok]

Hi, There's a prototype pollution vulnerability in .parse() related to the xml that are being parsed in it. In the following example the prototype pollution will affect the length parameter.

var plist = require('simple-plist');

var xml = `
<plist version="1.0">
    <key>metadata</key>
    <dict>
      <key>bundle-identifier</key>
      <string>com.company.app</string>
    </dict>
  </plist>`;

console.log(plist.parse(xml));
/**
 * * * * * * * * * * * * * * * * * * * * * * * * * *
 * * * * END OF THE NORMAL CODE EXAMPLE! * * * * * * 
 * * * * * * * * * * * * * * * * * * * * * * * * * * 
 **/


/**
 * * * * * * * * * * * *
 * PROTOTYPE POLLUTION *
 * * * * * * * * * * * *
 **/
var xmlPollution = `
<plist version="1.0">
  <dict>
    <key>__proto__</key>
    <dict>
      <key>length</key>
      <string>polluted</string>
    </dict>
  </dict>
</plist>`;
console.log(plist.parse(xmlPollution).length); // polluted

More information about the vulnerability: https://github.com/HoLyVieR/prototype-pollution-nsec18/blob/master/paper/JavaScript_prototype_pollution_attack_in_NodeJS.pdf

[SimenB]

Since GHSA-gff7-g5r8-mg8m is a thing now, maybe a ping to @wollardj could get this fixed? 😀

[Sujay-shetty]

could you please update plist package to latest version (3.0.5) where this vulnerability is fully fixed?
TooTallNate/plist.js#114

[wollardj]

I'm re-opening this for a bit. I want to write some tests to go along with this before I cut a new release, but I won't have time until later this evening.

[SimenB]

Seems weird the advisory points to this module if the bug is in a dependency...

[wollardj]

I've just published v1.3.1 to npm (https://www.npmjs.com/package/simple-plist/v/1.3.1) and tagged it as latest. Let me know if anyone sees any issues or if any additional audits are failing.

[srithar21]

Hi ,

I am getting the same issue in 1.3.1 version.

https://security.snyk.io/vuln/SNYK-JS-SIMPLEPLIST-2413671

[wollardj]

@srithar21 - I'm not sure how they're able to do that. I attempted to verify their POC by adding a test for it and the underlying plist.js library throws when it detects a __proto__ key.

Less important, but also a little suspicious, their POC wouldn't work anyway because the first byte is an unexpected ASCII sequence which would have made simple-plist attempt to use the binary parser. Since the file isn't binary, their example results in a different error with the message "Unable to determine format for plist aStringOrBuffer".

It's entirely possible if not likely that their POC is suffering from a formatting problem, but it's still worth noting since the POC doesn't appear to be valid.

Happy to be wrong about this if someone can point out something that I might be missing.

[oanatinus]

I wonder if you need to close this GitHub issue in order to trigger the various vulnerability databases to "recheck" the vulnerability...

[elopezanaya]

Hello, does someone know the status of this issue?, it seems like it was already solved , but this issue still is open, and in all CVE references is marked as not patched

[MaeveOReilly]

npm audit still listing it as open with no patch for me; I'm guessing @oanatinus is correct and this issue needs to be closed?

[oanatinus]

@wollardj what do you think, do you want to try closing this issue to see if that triggers the vulnerability databases to mark 1.3.1 as the fixed version?

[wollardj]

It's worth a shot

[brice-noowu]

Still an issue here, npm audit still listing it as open with no patch

[darakian]

It's worth a shot

In the future give us a shout over at https://github.com/github/advisory-database/ if we're out of date 😉