Closed Bug 1276976 (CVE-2016-9076) Opened 4 years ago Closed 4 years ago

<select> dropdown menu can be used for URL bar spoofing on e10s

Categories

(Core :: Layout: Form Controls, defect)

49 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla50
Tracking Status
e10s m9+ ---
firefox49 --- affected
firefox50 --- fixed

People

(Reporter: mats, Assigned: enndeakin)

References

Details

(Keywords: csectype-spoof, sec-moderate, testcase, Whiteboard: [post-critsmash-triage][e10s only][adv-main50+])

Attachments

(2 files, 1 obsolete file)

Attached file testcase
STR
1. load the attached testcase
2. click anywhere on the page background

ACTUAL RESULT
The dropdown menu opens upward, covering the URL bar.

EXPECTED RESULT
The dropdown menu should not cover the URL bar in any situation.

See bug 575294, bug 868327 for background on the problem and how we
fixed this on non-e10s.
Jessica, please CC any folks you think can help.  I don't know who are involved
in the e10s combobox implementation.  Thanks.
Flags: needinfo?(jjong)
So perhaps the simplest way to do this is to allow one to assign the constraint rectangle that nsMenuPopupFrame::GetConstraintRect will use, then it can be set from the browser frame area when opening the popup in BrowserParentHelper.jsm.
Group: core-security → layout-core-security
This is interesting, but I am complete new at this, maybe try :mconley? Neil would know better.
Flags: needinfo?(jjong)
One thing to note is that for the e10s select dropdown, <option> elements are prevented from showing images or doing any complex styling. We pay attention to their disabled and hidden states, but that's about it.

So it shouldn't be possible to put an image of a URL bar in the popup.

Am I right when I say that this reduces the exploit-ability of this somewhat?
Flags: needinfo?(mats)
> Am I right when I say that this reduces the exploit-ability of this somewhat?

Yes, somewhat.  If you manage to cover the URL bar text input somewhat precisely
then it might be hard for some users to detect though.  The URL bar chrome around
the text input is about the same shade of grey as the combobox dropdown,
at least on Linux.
Flags: needinfo?(mats)
Alright, well, the e10s team will triage this tomorrow.
Assignee: nobody → enndeakin
I just noticed that on my Mac that the positioning does not work in non-e10s either. The popup opens outside the content area and over the urlbar. Are there still issues there?
Flags: needinfo?(mats)
With e10s turned off, the testcase works fine for me in the latest Nightly on OSX.
Only one <option> is displayed in the dropdown menu, which is inside the content area.
Flags: needinfo?(mats)
OK, so it works in non-e10s on nightly builds. Perhaps this was improved in a recent change not mentioned in the initial comment. (1273129 maybe)
This patch adds the ability for a popup to have a custom constraint rectangle. SelectParentHelper.jsm uses the <browser> area for this.

The non-e10s case appears to fix this testcase to appear as a single item list with no scrolling mechanism. The popup code doesn't have knowledge of this so in e10s it appears slightly larger and the cropped off scrollbar area is still present.
This version fixes the constraining a bit to include the screen area vertically but not horizontally, as the non-e10s select does. It also adds a test.
Attachment #8760309 - Flags: review?(tnikkel)
Attachment #8759698 - Attachment is obsolete: true
Comment on attachment 8760309 [details] [diff] [review]
Allow setting the constraint rectangle for popups, v2

Seems reasonable.

>diff --git a/layout/xul/nsMenuPopupFrame.h b/layout/xul/nsMenuPopupFrame.h
>+  nsRect mOverrideConstraintRect;

Should we be initializing this in the nsMenuPopupFrame constructor?
Attachment #8760309 - Flags: review?(tnikkel) → review+
> 
> Should we be initializing this in the nsMenuPopupFrame constructor?

It should be initialized to 0 already, no?
Comment on attachment 8760309 [details] [diff] [review]
Allow setting the constraint rectangle for popups, v2

Need some dom reviewer to get around rule preventing checkin to dom/webibl
Attachment #8760309 - Flags: review?(mrbkap)
(In reply to Neil Deakin from comment #13)
> > 
> > Should we be initializing this in the nsMenuPopupFrame constructor?
> 
> It should be initialized to 0 already, no?

Oh right.
Attachment #8760309 - Flags: review?(mrbkap) → review+
sorry had to back this out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=29886186&repo=mozilla-inbound
Flags: needinfo?(enndeakin)
I accidentally included some changes from another bug so I'll check in the correct version in a bit.
Flags: needinfo?(enndeakin)
https://hg.mozilla.org/mozilla-central/rev/2ce7e838634b
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Group: layout-core-security → core-security-release
Flags: qe-verify-
Whiteboard: [e10s only] → [post-critsmash-triage][e10s only]
Whiteboard: [post-critsmash-triage][e10s only] → [post-critsmash-triage][e10s only][adv-main50+]
Alias: CVE-2016-9076
See Also: → 1328067
Can we un-hide this bug now?  The fix landed in Firefox 50, which I think means all supported versions have the fix.

ESR45 *might* have been affected, but:
 (a) it wasn't really affected because this bug is e10s-only, and e10s is disabled by default in ESR45 (with no UI-exposed way to enable it.   I just tried unsuccessfully to force-enable it in about:config)

 (b) Even if it were affected, ESR45 hit end-of-life ~6 weeks ago (on June 13th, according to the timeline-image on https://www.mozilla.org/en-US/firefox/organizations/faq/ )

So I don't think any real users can be harmed by this bug at this point.
Flags: needinfo?(abillings)
Group: core-security-release
Flags: needinfo?(abillings)
You need to log in before you can comment on or make changes to this bug.