Bug 1248329 (CVE-2016-2829)

Requesting permissions in short succession can lead to the microphone icon displayed for an unrelated notification

VERIFIED FIXED in Firefox 47

Status

()

Firefox
Device Permissions
VERIFIED FIXED
a year ago
5 months ago

People

(Reporter: Tim McCormack, Assigned: florian)

Tracking

({sec-low})

38 Branch
Firefox 47
sec-low
Points:
---

Firefox Tracking Flags

(firefox47 verified, firefox48 fixed, firefox49 verified)

Details

(Whiteboard: [adv-main47+])

Attachments

(5 attachments, 1 obsolete attachment)

(Reporter)

Description

a year ago
Created attachment 8719379 [details]
demo-6194b06b95d2c580b7.html

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Firefox/38.0 Iceweasel/38.6.1
Build ID: 20160214081214

Steps to reproduce:

I created a web page to request as many permissions as possible within a short span of time. (Geolocation, pointer lock, microphone and camera access, notifications.) Demo attached.


Actual results:

Under some conditions, the notifications dialogs have the wrong icon, e.g. microphone icon for notifications or (more worryingly) for geolocation.

This is probably a race condition. If it can be controlled, it could lead to a user "consenting" to the wrong permission. (Marking as Security bug out of caution for now, feel free to remove flag.)

The boxes also flash past in a way that indicates that sometimes one permission cancels display of another.


Expected results:

Notifications boxes should appear in quick succession (but do not replace each other) or form a single stack. They should also have the correct icons.
(Reporter)

Comment 1

a year ago
Created attachment 8719382 [details]
Demonstration of local (file://) wrong-icon behavior at demo commit 6194b06b95d2c580b7

Adding screenshot of behavior occurring when I view the demo locally via file://. This is fairly easy to reproduce; loading the page and then hitting F5 is usually sufficient. Since I'm triggering one of the permission requests on keyup, hitting spacebar in the page may be required before hitting F5.

I'm on Debian 7 (Wheezy) with XGCE, if it matters.
(Reporter)

Comment 2

a year ago
Created attachment 8719383 [details]
demo-6194b06b95d2c580b7-remote.png

Adding screenshot of behavior occurring over HTTPS at same demo commit. Much more difficult to reproduce than over file://.
(Reporter)

Comment 3

a year ago
Was also able to reproduce on Firefox 44 over file://; have not confirmed over https. Race condition harder to trigger, or different in permissions is changing things.

Comment 4

a year ago
I expect this is an issue with PopupNotifications.jsm, but I'm not sure. Florian, have you seen something like this before?
Component: Untriaged → Notifications and Alerts
Flags: needinfo?(florian)
Product: Firefox → Toolkit
(Assignee)

Comment 5

a year ago
The problem seems to be specifically with the microphone icon, which isn't very surprising, as that icon has specific code. The notification for camera (or camera+microphone) and microphone is the same, and the icon to show in the panel is selected at the time the popup is shown.

The code doing this is at http://hg.mozilla.org/mozilla-central/annotate/e355cacefc88/browser/modules/webrtcUI.jsm#l361 and I would guess a fix involves replacing ".firstChild" on that line.

I didn't know it was possible to have several notifications shown at once, is this a recent change related to the permission center?
Flags: needinfo?(florian)

Comment 6

a year ago
(In reply to Florian Quèze [:florian] [:flo] from comment #5)
> The problem seems to be specifically with the microphone icon, which isn't
> very surprising, as that icon has specific code. The notification for camera
> (or camera+microphone) and microphone is the same, and the icon to show in
> the panel is selected at the time the popup is shown.
> 
> The code doing this is at
> http://hg.mozilla.org/mozilla-central/annotate/e355cacefc88/browser/modules/
> webrtcUI.jsm#l361 and I would guess a fix involves replacing ".firstChild"
> on that line.

OK, moving the bug accordingly, then. :-)

> I didn't know it was possible to have several notifications shown at once,
> is this a recent change related to the permission center?

Considering this was reported against ESR, I would not expect so.
Component: Notifications and Alerts → Device Permissions
Product: Toolkit → Firefox
(Assignee)

Updated

a year ago
Summary: Requesting web page permissions in short succession can lead to the wrong icon displayed for a permission → Requesting permissions in short succession can lead to the microphone icon displayed for an unrelated notification
(Reporter)

Comment 7

a year ago
Created attachment 8719640 [details]
Demo v2, deterministic

Attaching a test case that is deterministic, using Florian's discovery. Clicking the demo button brings up three requests in quick succession, microphone second. Each preempts any existing dialog. Cancelling the third one shows the first two stacked, and the icon problem appears reliably.
Assignee: nobody → florian
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 8

a year ago
Created attachment 8719852 [details] [diff] [review]
Patch
Attachment #8719852 - Flags: review?(gijskruitbosch+bugs)

Comment 9

a year ago
Comment on attachment 8719852 [details] [diff] [review]
Patch

Review of attachment 8719852 [details] [diff] [review]:
-----------------------------------------------------------------

TBH, could just use getElementById here - Firefox code really shouldn't have multiple items with the same ID, and hopefully there are tests for this working? :-)
Attachment #8719852 - Flags: review?(gijskruitbosch+bugs) → review+
Keywords: sec-low
(Assignee)

Comment 10

a year ago
Created attachment 8720216 [details] [diff] [review]
Patch v2

Using getElementById instead of querySelector.

(In reply to :Gijs Kruitbosch from comment #9)
> hopefully there are tests for this working? :-)

http://mxr.mozilla.org/mozilla-central/source/browser/base/content/test/general/browser_devices_get_user_media.js?rev=43a902e35df7#67 covers the microphone icon being shown.
(Assignee)

Updated

a year ago
Attachment #8719852 - Attachment is obsolete: true
(Assignee)

Comment 11

a year ago
Comment on attachment 8720216 [details] [diff] [review]
Patch v2

Carrying forward Gijs' r+ from comment 9.
Attachment #8720216 - Flags: review+
(Assignee)

Comment 12

a year ago
https://hg.mozilla.org/integration/fx-team/rev/a8cd1f16ed6411dfe8e768596f3e594760050235
Bug 1248329 - Requesting permissions in short succession can lead to the microphone icon displayed for an unrelated notification, r=Gijs.
https://hg.mozilla.org/mozilla-central/rev/a8cd1f16ed64
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox47: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
Group: firefox-core-security → core-security-release
Whiteboard: [adv-main47+]
Alias: CVE-2016-2829
Reproduced the issue on FX 46.0.1 using the second testcase.
Verified fixed FX 47 RC, 49.0a1 (2016-06-01) on Win 7.
Status: RESOLVED → VERIFIED
status-firefox47: fixed → verified
status-firefox49: --- → verified
status-firefox48: --- → fixed
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.