We sandbox reader mode content, so this should already protect us from content pages. As an added safety precaution, though, we can try sanitizing pages when they're viewed to prevent any scripts from running in the first place.

If you can stick the page in an <iframe>, you might be able to use iframe sandbox without 'allow-scripts' (and probably without 'allow-same-origin' too which would further help), which will disable plugins also. I'll paste dveditz's comment from bug 778582 comment 2 : "The only safe way is to use the same parser used by the browser itself. Look into nsIParserUtils, or ask the content folks what to do if that's not suitable (particularly hsivonen). Parsing by regexp is another red flag. There are zillions of ways to encode HTML to evade parsers, see all the XSS even on sites that are trying to be careful. The right sanitizer would have stripped out the event handlers." I will echo that sanitizing HTML is very, very tricky.

Comment on attachment 657066 [details] [diff] [review] Sanitize parsed reader mode article when viewing Do we have any data on how this change affects performance and memory?

Comment on attachment 657066 [details] [diff] [review] Sanitize parsed reader mode article when viewing I'd still like to see some before/after speed and memory results

(In reply to Mark Finkle (:mfinkle) from comment #5) > Comment on attachment 657066 [details] [diff] [review] > Sanitize parsed reader mode article when viewing > > I'd still like to see some before/after speed and memory results On a more powerful device like the Galaxy Nexus, the parseFragment() call takes up to 4ms. On a less powerful device like the Galaxy S, it takes up to 5ms. Still need to check memory usage, but given that the article content is a fairly simple HTML (it's usually just a DIV with some Ps inside) I wouldn't expect it to cause any relevant change.

Used the memory reporter to track memory usage and couldn't see any jumps in memory usage when sanitizing the article content. Pushed: https://hg.mozilla.org/integration/mozilla-inbound/rev/18155f074640

Comment on attachment 657066 [details] [diff] [review] Sanitize parsed reader mode article when viewing [Approval Request Comment] User impact if declined: Pages with javascript handlers might still be able to run javascript code inside reader. Although the about:reader is unprivileged, the code could still change the page content which is undesirable. Testing completed (on m-c, etc.): Local testing, no performance regressions, no issues found. Landed in m-c. Risk to taking this patch (and alternatives if risky): Very low, simply sanitizing the article content before showing it in reader. String or UUID changes made by this patch: none.

Comment on attachment 657066 [details] [diff] [review] Sanitize parsed reader mode article when viewing low risk and early enough in Beta still that we can take this sanitizing patch.

Pushed to aurora: https://hg.mozilla.org/releases/mozilla-aurora/rev/b9cd614bedaa Pushed to beta: https://hg.mozilla.org/releases/mozilla-beta/rev/d47f00b3ad36