1276976 - (CVE-2016-9076) <select> dropdown menu can be used for URL bar spoofing on e10s

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, defect)

Description

[Mats Palmgren (inactive)]

Attachment: 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.

Comment 1

[Mats Palmgren (inactive)]

Jessica, please CC any folks you think can help.  I don't know who are involved
in the e10s combobox implementation.  Thanks.

Comment 2

[Neil Deakin]

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.

Comment 3

[Jessica Jong [:jessica] (inactive)]

This is interesting, but I am complete new at this, maybe try :mconley? Neil would know better.

Comment 4

[Mike Conley (:mconley) (:⚙️)]

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?

Comment 5

[Mats Palmgren (inactive)]

> 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.

Comment 6

[Mike Conley (:mconley) (:⚙️)]

Alright, well, the e10s team will triage this tomorrow.

Comment 7

[Neil Deakin]

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?

Comment 8

[Mats Palmgren (inactive)]

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.

Comment 9

[Neil Deakin]

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)

Comment 10

[Neil Deakin]

Attachment: Allow setting the constraint rectangle for popups

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.

Comment 11

[Neil Deakin]

Attachment: Allow setting the constraint rectangle for popups, v2

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.

Comment 12

[Timothy Nikkel (:tnikkel)]

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?

Comment 13

[Neil Deakin]

> 
> Should we be initializing this in the nsMenuPopupFrame constructor?

It should be initialized to 0 already, no?

Comment 14

[Neil Deakin]

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

Comment 15

[Timothy Nikkel (:tnikkel)]

(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.

Comment 16

[Neil Deakin]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e7a3ba795e1a1f0547505e2c240baef183243fd3
Bug 1276976, ensure that popups are visible in the right area, r=tn, r=mrbkap

Comment 17

[Carsten Book [:Tomcat]]

sorry had to back this out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=29886186&repo=mozilla-inbound

Comment 18

[Neil Deakin]

I accidentally included some changes from another bug so I'll check in the correct version in a bit.

Comment 19

[Neil Deakin]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2ce7e838634b204dcec9487452cabc20d709c4d6
Bug 1276976, ensure that popups are visible in the right area, r=tn, r=mrbkap

Comment 20

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2ce7e838634b

Comment 21

[Daniel Holbert [:dholbert]]

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.