Update Validator Regular Expression for prototype



9 years ago
3 years ago


(Reporter: sdwilsh, Unassigned)





9 years ago
Matched Pattern: "/(Object|Array|String|Number|Boolean|Date)\.prototype/" catches Array.prototype.slice.call(arguments, 2).  This code is perfectly safe.  The regular expression should probably be:

We want to catch an add-on modifying the prototype.  Calling a function off of the prototype is a way to defend against add-ons that modify the prototype and should not be discouraged.
Our RE's are pretty general to try to avoid all false negatives. I can think of a few possible manipulations of prototypes that wouldn't involve having a = immediately afterwards. Plus, I tested your proposed RE and it still matches the case you want to filter out (because of the last ?)

Comment 2

9 years ago
I suppose that using a function to pass in Array.prototype, and then modifying it in the function would do that.  We would want to drop that last ? (I think I forgot to delete it when I rewrote part of the bit I added).  Can we whitelist certain expressions so that if it's flagged, but in the whitelist, we don't error?
The current code is extremely simple, it matches the RE and if it does, it shows a warning. 

Line 575

I don't think it's worth adding code just to filter out one case where it's evident that it's not a problem. I'd rather filter out all of the setTimeout/setInterval false positives, that really pollute the results. I'd be willing to improve the RE in order to make it more precise, but I don't see how that could be done.
-> incomplete.  Reopen if there is something to do here.
Last Resolved: 8 years ago
Resolution: --- → INCOMPLETE


3 years ago
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.