isaacs / sax-js

A sax style parser for JS
Other
1.09k stars 325 forks source link

Is this code safe against various XML attacks? #243

Closed sjlongland closed 5 years ago

sjlongland commented 5 years ago

Hi all,

I'm just looking at implementing a XML-RPC server (using express-xmlrpc) for a project. My reasoning for choosing this over doing it myself using JSON is that some of the code I may be interfacing with will be in VisualBASIC 6 (yes, I know), and dealing with JSON in such an environment is a pain, whereas there's good libraries for doing XML and XML-RPC which make the process rather trivial. As a bonus, Python, PHP and other languages can deal with it easily, so from an integration perspective, we get a lot "for free".

express-xmlrpc uses this library for its XML parsing and I was wondering if there were any known issues regarding the safety of this XML parser parsing arbitrary input? (e.g. vulnerabilities to XML parser attacks like billion laughs, etc.)

https://www.npmjs.com/package/sax#regarding-doctype-s-and-entity-s seems to suggest that it would be safe since the <!ENTITY …> blocks are not parsed by this library (that's left as an exercise to the consumer of the library, which express-xmlrpc does not appear to do).

I'd be interested to know if anyone knows of any hidden gotchas that I need to be wary of.

isaacs commented 5 years ago

I am not aware of any security problems in this library. Of course, if I were aware of them, I would have fixed them :) (Or I put them there, and I'm malicious, and you can't trust what I'm saying anyway.)

As you mention, a lot of the XML surface area was not implemented, in the interest of keeping the module simple. I'm often surprised that so many people find this thing useful after all these years, since it really isn't the fasted or most full-featured XML parser, but perhaps that's part of its strength. XML is so large when fully implemented that it's very hard to secure it against attack.

It has not undergone a rigorous penetration test, so you're definitely using it at your own risk. I'm fairly confident it's not terrible, and if you do find anything, I'll be happy to fix it promptly. (Please report by emailing i@izs.me, not posting publicly prior to a fix being deployed.) There's also a chance that express-xmlrpc has a vulnerability entirely unrelated to XML parsing, so keep that in mind.