652848 - Validator can be bypassed using string manipulation, like window['set'+ 'Timeout']

Status: RESOLVED FIXED

(addons.mozilla.org Graveyard :: Developer Pages, defect)

Description

[Jorge Villalobos [:jorgev] (he/him)]

Attachment: Add-on version with bypass code

The attached file demonstrates how to bypass some validator flags, specifically the setTimeout flag. On file chrome/content/azan.js, you'll see the following:

timeoutID = window['set'+ 'Timeout'](azan.run, 1000);

In this case the bypass is harmless and just an innocent attempt to clear some warnings, but it could be problematic if done with more sensitive flags. We need to make the validator recognize these patterns and show the right flags.

Comment 1

[Matt Basta [:basta]]

Does this actually bypass the current validator? It shouldn't we have tests already for things like

window["ev"+"al"]

This is done through the lazy evaluation of the script. If it's not being detected, I'd imagine that the problem is likely a more general issue that's preventing an error from being raised.

I'll look into it soon.

Comment 2

[Matt Basta [:basta]]

There was a minor bug in the MemberExpression evaluator. It should be fixed here:

https://github.com/mattbasta/amo-validator/commit/56930d91ea199322a784528ba3de3ca9d686ad9c

Comment 3

[Matt Basta [:basta]]

Merged:

https://github.com/mozilla/amo-validator/commit/51115834a5f1d7d87a62fd9ed3a3e287b71bc4a5

Comment 4

[Jorge Villalobos [:jorgev] (he/him)]

Reclassifying editor bugs and changing to a new whiteboard flag. Spam, spam, spam, spam...