531454 - Validator should flag usage of js-ctypes

Status: RESOLVED FIXED

(addons.mozilla.org Graveyard :: Admin/Editor Tools, enhancement, P4)

Description

[Justin Dolske [:Dolske]]

The addon validator should probably flag usage of js-ctypes... EG, check for the string "ctypes.jsm".

JS-ctypes is new functionality added in Firefox 3.6 to allow javascript to call APIs that previously required writing native code. This is generally a good thing, as it allows addons to do things in pure JS that previously required inclusion of mysterious binary components.

However, using js-ctypes still carries some of the risks of binary components -- even though it's in JS, if you write sloppy code you can end up with crashes and security holes (eg, buffer overflows). I don't think an addon using js-ctypes should be immediately rejected from AMO, but it certainly merits closer inspection of the code.

See: https://developer.mozilla.org/en/JavaScript_code_modules/ctypes.jsm

Comment 1

[Wil Clouser [:clouserw]]

-> jorge for decision.  If we want to add this, give me parameters like what files to look in (can be all) and what to look for (a regex).

Comment 2

[[e-mail]]

I'd keep it simple: all files for the string "ctypes.jsm".

Comment 3

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

Check all Javascript files (.js, .jsm, other?) for any of the following strings:
ctypes.jsm
@mozilla.org/jsctypes

All add-ons with this flag will need to have admin reviews (yay), so the message should make it clear that using js-ctypes is not unlike including binary code in the add-on.

Comment 4

[[e-mail]]

Does this also mean that it would be easy to get a list of addons that are using ctypes? This will probably be useful information to me at some point, since the API is still in development and real-world usage is important when deciding the impact of changes to it.

Comment 5

[Wil Clouser [:clouserw]]

(In reply to comment #4)
> Does this also mean that it would be easy to get a list of addons that are
> using ctypes? This will probably be useful information to me at some point,
> since the API is still in development and real-world usage is important when
> deciding the impact of changes to it.

No, you are looking for bug amo-mxr (will that auto-link?  amo-mxr is the bugs alias).

I'm assigning this to RJ in case he is bored.  RJ:  If you're busy, you can unassign yourself.

Comment 6

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

On second thought, better check all files. I can see this possibly showing up on a XUL or XBL file, and it's unlikely to cause false positives anyway.

Comment 7

[RJ Walsh [:rjwalsh]]

Attachment: Tests

This is just adding these to the Unsafe JavaScript test - we could add this to a new test if that would make more sense.

Comment 8

[Wil Clouser [:clouserw]]

Comment on attachment 418336 [details] [diff] [review]
Tests

wfm

Comment 9

[RJ Walsh [:rjwalsh]]

Fixed in r58195