Prevent prototype pollution in $.deparam#61
Conversation
- using YUI compressor
|
@constancecchen This looks like a good change; I had some trouble getting this to pass the unit tests though. Could you try running those locally? (it should be a case of opening the |
|
@constancecchen A small reminder about the unit tests here, if & when you have time - or let me know if you're likely to be too busy to take more of a look. |
|
Super sorry about the slow response!! I've been swamped and totally procrastinated on this. It would be awesome to get some help on the unit tests, if anyone's around. For what it's worth also we ran into another issue with prototype pollution even with this PR: The issue there is when an obj is initially defined as a non-object (e.g. a string or an array), the prototype of the prototype of that string/array can then by still be polluted (yay, JS). I think the issue is somewhere around line 521 of the code but I could be wrong. With that in mind a more blunt-force solution instead of |
|
@cee-chen This issue was fixed like you suggested in LimeSurvey: LimeSurvey/LimeSurvey@0c4651b |
See #62 for a more in-depth description of how to reproduce the prototype pollution.
Using
Object.create(null)prevents prototype pollution because the objects it creates have no prototype/constructor, and therefore does not pollute the global prototype.You can test/confirm this for yourself by comparing the outputs of:
Object.create(null).__proto__.test = 'polluted'(should simply fail)({}).__proto__.test = 'polluted'jQuery/browser compatability
As a reminder, if you fork this or care about prototype pollution in jQuery BBQ, you must also be on jQuery 3.4.0+, otherwise you're still vulnerable via jQuery's
$.extend.Please also note that
Object.create(null)is only supported on IE9+. This fork therefore drops support for IE6-8 (which should be the case anyway if you are on jQuery 3.4.0, which does not support$.browser).