779007 - Audit our privileged about: pages

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[:Margaret Leibovic]

A lot of our about: pages are privileged, and it seems like some of them don't need to be (e.g. about:certerror, about:blocked).

I'm also unsure why we're not using the URI_SAFE_FOR_UNTRUSTED_CONTENT flag for unprivileged pages. I think it would be worthwhile to just make sure we're being as safe as can be here.

Comment 1

[:Gavin Sharp [email: [e-mail]]]

Not using URI_SAFE_FOR_UNTRUSTED_CONTENT for unprivileged pages is weird, but isn't a security issue (it just means none of the pages are linkable from untrusted content).

Having pages that can be loaded (indirectly) by content be privileged *is* a security issue, though. about:home (via window.home()), about:certerror/blocked (via embedding a phishing/malware page, or page with a bad cert) are in this category.

Comment 2

[:Margaret Leibovic]

Attachment: don't make about: pages privileged if they don't need to be

I looked in blockedSite.xhtml, aboutCertError.xhtml, and aboutHome.xhtml, and none of these pages try to make any privileged calls.

Unlike desktop, our about:/about:firefox pages have the update install stuff, but that's okay because external pages can't link to it.

Comment 3

[:Margaret Leibovic]

Attachment: don't make about: pages privileged if they don't need to be

Whoops wrong patch.

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 647579 [details] [diff] [review]
don't make about: pages privileged if they don't need to be

blockedSite and aboutCertError use "click" events handled in browser.js, which should still work fine with this change.

Comment 5

[:Margaret Leibovic]

https://hg.mozilla.org/integration/mozilla-inbound/rev/89fb39735ee3

Comment 6

[:Margaret Leibovic]

Comment on attachment 647579 [details] [diff] [review]
don't make about: pages privileged if they don't need to be

[Approval Request Comment]
Bug caused by (feature/regressing bug #): n/a
User impact if declined: possible security risk from content indirectly loading privileged about pages
Testing completed (on m-c, etc.): just landed on inbound
Risk to taking this patch (and alternatives if risky): low-risk, changing the privilege level of some pages to match desktop
String or UUID changes made by this patch: n/a

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/89fb39735ee3

Is it possible to test this?

Comment 8

[:Margaret Leibovic]

(In reply to Ryan VanderMeulen from comment #7)
> https://hg.mozilla.org/mozilla-central/rev/89fb39735ee3
> 
> Is it possible to test this?

We don't have any automated test for this AFAIK. On desktop you can use the web console to try to execute privileged code while on an about: page (e.g. try running |Components.stack|), but this patch only affects mobile, so I'm not sure of the best way to test it. I think we can trust that setting this privileged flag does the right thing for these pages. Or otherwise we'd have a more serious problem!

Gavin, do you have an idea of how to test this manually?

Comment 9

[:Gavin Sharp [email: [e-mail]]]

I don't think there's an easy way to test this manually in Fennec, short of exploiting one of our open security vulnerabilities :)

Comment 10

[Brian Nicholson (:bnicholson)]

We might be able to create iframes containing the about pages, and use their windows as a sandbox context. On some chrome:// page, try this:

// no error
let iframe = document.createElement("iframe");
iframe.setAttribute("src", "about:config");
document.body.appendChild(iframe);
let sandbox = Components.utils.Sandbox(iframe.contentWindow);
Components.utils.evalInSandbox("Components.classes", sandbox);

// Error: Permission denied for <moz-safe-about:blank> to get property XPCComponents.classes
iframe = document.createElement("iframe");
iframe.setAttribute("src", "about:blank");
document.body.appendChild(iframe);
let sandbox = Components.utils.Sandbox(iframe.contentWindow);
Components.utils.evalInSandbox("Components.classes", sandbox);

Comment 11

[:Gavin Sharp [email: [e-mail]]]

Oh, yes - I was assuming a "without changing code" constraint. It's certainly possible to test that the page isn't privileged by changing its code (or injecting code into it programmatically, etc.), but I assumed Margaret had done that locally already.

Comment 12

[:Margaret Leibovic]

https://hg.mozilla.org/releases/mozilla-aurora/rev/998b5d096f2c
https://hg.mozilla.org/releases/mozilla-beta/rev/60f195931a12