Recognize certain jQuery methods in validator

NEW
Unassigned

Status

addons.mozilla.org Graveyard
Add-on Validation
P2
major
7 years ago
a year ago

People

(Reporter: kmag, Unassigned)

Tracking

Details

(Whiteboard: [ReviewTeam:P1])

User-Agent:       Mozilla/5.0 (X11; Linux x86_64; rv:6.0a1) Gecko/20110419 Firefox/6.0a1
Build Identifier: 

As much as I hate to admit it, jQuery is fantastically popular among add-on authors. I think it would be a good idea to recognize certain jQuery methods and process them in the same manner as the equivalent DOM methods. The two that come to mind at the moment are .html() which should be processed the same way as the .innerHTML property, and .attr() which should be processed roughly the same as setAttribute (although when given an object literal, that might be a bit difficult). I'm sure that there are others that would be of interest as well.

It's probably best to just process these as universal method calls. It seems to me that it would be more trouble than it's worth to try to track which objects are jQuery proxies.

Reproducible: Always
Assignee: nobody → mbasta
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P3
Whiteboard: [required amo-editors]
Target Milestone: --- → Q2 2011

Comment 1

7 years ago
That would probably be easiest. I could just use alises in instanceactions and instanceproperties. I think.

Comment 2

7 years ago
I'd imagine that we should also support .prop() for jQuery 1.6. Just using this as a note to myself...
After a cursory look over the API docs, these are the only other methods that leap out as especially dangerous (and they're probably rather more dangerous than the above):

http://api.jquery.com/jQuery.globalEval/
http://api.jquery.com/jQuery.getScript/
http://api.jquery.com/load/
Ah, alas, I've just run into an add-on which uses the following. Apparently they accept HTML strings as well as elements:

http://api.jquery.com/after/
http://api.jquery.com/append/
http://api.jquery.com/before/
http://api.jquery.com/prepend/

And I recall that you can do things like $("<html>...</html>"), jQuery("<html>...</html>"). I don't expect that there are sane ways to handle all of these. They'll probably just have to be ignored, but I guess they're worth mentioning nonetheless.

Comment 5

7 years ago
I just added the first three. With regard to the last three, we can just run the input through a filter similar to what we have for innerHTML. For $ as a function, we'd do the same thing but test the inside with a Regex for whether it's an HTML string or not.
Target Milestone: Q2 2011 → Q3 2011
This is pretty important, as it can lead to dangerous code execution. 

Here's an update from the mailing list. We need to detect the above, plus 'jQuery.get, jQuery.post, or jQuery.ajax with a data type containing "script" or "jsonp"'.
Severity: normal → major
Priority: P3 → P2

Comment 7

7 years ago
jQuery will automatically switch to JSONP mode if there is a question mark in the URL of an AJAX request after the GET parameter list has been started. I.e.:

http://foo.com/?callback=?

We should test for this, too, as it's much more common with jQuery developers.
Is it? Ugh... I dislike jQuery more every day. I gave up trying to make sense of the code that handles XHRs after about five minutes. I wouldn't be surprised if there were dozens of ways for it to execute code that I don't know about.

Comment 9

7 years ago
I feel like this is going to be one hell of a patch.
These are the preliminary runtime tests that I'm experimenting with: http://code.google.com/p/extension-test/source/browse/extension/modules/jquery-wrapper.jsm

They might be marginally useful to you, but I suspect that the checks will be much more difficult statically...
Assignee: mattbasta → nobody
Component: Admin/Editor Tools → Add-on Validation
QA Contact: admin-tools → add-on-validation
Assignee: nobody → mattbasta
Target Milestone: Q3 2011 → Q1 2012
Reclassifying editor bugs and changing to a new whiteboard flag. Spam, spam, spam, spam...
Whiteboard: [required amo-editors] → [ReviewTeam]
Resetting a bunch of missed milestones. Sorry for the bugspam.
Target Milestone: Q1 2012 → ---
Assignee: mattbasta → kmaglione+bmo
Whiteboard: [ReviewTeam] → [ReviewTeam:P1]

Comment 13

3 years ago
While Jquery is unique in its de facto across the web and add-ons, if another Library is used such as one from the following incomplete list: Wouldn't the main concern and problems still persist? 

ExtJS
YUI 
DHTMLX
Script.aculo.us
PrototypeUI
MochiKit
LivePipe UI
UKI 
AmpleSDK 
Modernizr

-Null
(Assignee)

Updated

2 years ago
Product: addons.mozilla.org → addons.mozilla.org Graveyard
Assignee: kmaglione+bmo → nobody
You need to log in before you can comment on or make changes to this bug.