security: regex with potential catastrophic backtracking

[davisjam]

Commit 74d5395 introduced a vulnerable regexp, which on malicious input can take minutes or years to evaluate.

It currently appears in files: ["dist/plist.js","dist/plist-build.js"]

Both contain this code:

XMLStringifier.prototype.xmlEncoding = function(val) {
  val = '' + val || '';
  if (!val.match(/^[A-Za-z](?:[A-Za-z0-9._-]|-)*$/)) {
    throw new Error("Invalid encoding: " + val);
  }   
  return val;
};  

Here is an attack string (JSON-formatted):

{"pumpPairs":[{"pump":"-A","prefix":"AA"}],"suffix":"\u0000"}

A string composed of the indicated prefix, the "pump" concatenated 30 times, and then the suffix, takes about 10 seconds to evaluate on my machine, and will double for each additional concatenation of the pump string.

I believe this regex is due to this bundled library.

I disclosed this vulnerability to the author of xmlbuilder-js and v9.0.7 has the fix.
Here's the commit. He removed the '|-' from the quantified group.

Can you re-build to pull in the new version of the library?
Alternatively, you can just spot-check the regex he changed.

[mreinstein]

I think the solution is probably to upgrade xmlbuilder to 9.x The issue here is that xmlbuilder dropped support for node < v6 and it would have a similar impact on plist.js. I suppose we could follow suit and just push a plist v3 which does the same thing.

I lament having to deprecate all these node versions which work fine otherwise, just because deps are dropping support. Such is the world we live in now I suppose. @TooTallNate what are your thoughts?

[davisjam]

FWIW, Node v4 reaches end of life at the end of April.
Node release schedule.

[mreinstein]

I understand that node v4 is considered end of life by node's standards, but it's also not that old in the grand scheme of things. I mean the LTS is less than 2 years. That is comically short for something called "long term". It's a tradeoff between moving fast and having stability. I appreciate the high velocity but on the other hand, this module despite it's problems works fine on most versions of node. I hate the idea of forcing it to break on older version of node just because the community wants to be on a fast moving upgrade treadmill.

[davisjam]

We are on the same page about Node's "LTS".

If you don't want to upgrade xmlbuilder, you can duplicate the fix here by adopting the new regex. Though depending on how you build plist, this may be asking for trouble later.

[mreinstein]

I'm not totally opposed to simply moving past old node and staying on the upgrade hamster wheel, it just makes me a little sad to see older versions of software simply broken for the sake of being old. :-/

I'll wait and see what Nate thinks.