Audit our privileged about: pages

RESOLVED FIXED in Firefox 15

Status

()

Firefox for Android
General
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Margaret, Assigned: Margaret)

Tracking

({sec-audit})

Trunk
Firefox 17
ARM
Android
sec-audit
Points:
---
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(firefox15 fixed, firefox16 fixed, firefox-esr10 unaffected)

Details

(Whiteboard: [advisory-tracking-])

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

6 years ago
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.
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.
tracking-fennec: --- → ?
(Assignee)

Comment 2

6 years ago
Created attachment 647578 [details] [diff] [review]
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.
Assignee: nobody → margaret.leibovic
Attachment #647578 - Flags: review?(mark.finkle)
(Assignee)

Comment 3

6 years ago
Created attachment 647579 [details] [diff] [review]
don't make about: pages privileged if they don't need to be

Whoops wrong patch.
Attachment #647578 - Attachment is obsolete: true
Attachment #647578 - Flags: review?(mark.finkle)
Attachment #647579 - Flags: review?(mark.finkle)
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.
Attachment #647579 - Flags: review?(mark.finkle) → review+
Keywords: sec-audit
(Assignee)

Comment 5

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/89fb39735ee3
Target Milestone: --- → Firefox 17
(Assignee)

Comment 6

6 years ago
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
Attachment #647579 - Flags: approval-mozilla-beta?
Attachment #647579 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/89fb39735ee3

Is it possible to test this?
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
(Assignee)

Comment 8

6 years ago
(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?
I don't think there's an easy way to test this manually in Fennec, short of exploiting one of our open security vulnerabilities :)
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);
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.
Attachment #647579 - Flags: approval-mozilla-beta?
Attachment #647579 - Flags: approval-mozilla-beta+
Attachment #647579 - Flags: approval-mozilla-aurora?
Attachment #647579 - Flags: approval-mozilla-aurora+
(Assignee)

Comment 12

6 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/998b5d096f2c
https://hg.mozilla.org/releases/mozilla-beta/rev/60f195931a12
tracking-fennec: ? → ---
status-firefox15: --- → fixed
status-firefox16: --- → fixed
Whiteboard: [advisory-tracking-]
(Assignee)

Updated

6 years ago
Depends on: 781091
status-firefox-esr10: --- → unaffected
Group: core-security
You need to log in before you can comment on or make changes to this bug.