fixed Prototype Pollution using .parse() #114

[ghost]

Fixed issue #114 Prototype Pollution using .parse()
Please review and merge.

[Enet4]

I stumbled upon this by chance, but I ought to warn that this does not really fix #114. As described in the issue, the parsed output can have its prototype reassigned, which leads to the apparent surge of properties in the root object. Luckily, this seems to create new objects for the prototype every time. It does not seem to be able to pollute the prototypes of other objects.

---

The proposed patch never does anything, because key is always a string, so it is never equal to the global __proto__ .

The given test also passes regardless of whether the code is patched. A better set of assertions would have been the following:

var parsed = parse(xml);
assert.notEqual(parsed.length, 'polluted');
assert.notEqual(new Object().length, 'polluted');

The second assertion passes with or without the given change to the parser, since there's no real prototype pollution going on, but the first one does not. It can only pass if the implementation is adjusted to ignore the __proto__ key, thus making it not supported. With additional breaking changes to the API, an alternative is to replace dictionary objects with Maps.

[Enet4]

deepEqual does not test the objects' prototypes, which is why it passes. A deepStrictEqual test would fail, but we could never make it pass. A better test would check that parsed.length is not provided through the prototype chain.

Suggested change

	assert.deepEqual(parsed, {
	__proto__: {
	length: 'polluted'
	}
	});
	assert.notEqual(parsed.length, 'polluted');

[mreinstein]

resolved via #118