New issue
Advanced search Search tips
Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug-Security
Team-Security-UX



Sign in to add a comment
link

Issue 816033: Security: Permission request UI spoof

Reported by chromium...@gmail.com, Feb 24 2018

Issue description

Chrome Version: 66.0.3353.0 
Operating System: Only on Windows

I believe this is similar to  bug 774438 .

1. Set up a local webserver to host poc.html
2. Click on "Click here" button 
3. Observe the permission request stays open after navigation to another origin (with http://localhost wants to...)
 
Screen Shot 2018-02-24 at 16.03.46.png
124 KB View Download
poc.html
314 bytes View Download

Comment 1 by elawrence@chromium.org, Feb 25 2018

Cc: raymes@chromium.org
Components: UI>Browser>Permissions>Prompts
Labels: FoundIn-66 OS-Windows

Comment 2 by raymes@chromium.org, Feb 26 2018

Cc: timloh@chromium.org
Owner: guidou@chromium.org
Status: Assigned (was: Unconfirmed)
This repros on stable M64 on linux for me.

It looks like 2 requests are being queued in the PermissionBubbleMediaAccessHandler and the second one is still being run even after the navigation happens. I know there is logic to cancel media requests after a navigation happens so there must be a missing case somewhere. To be safe we should check that the origin of the request matches the origin of the current frame. We could put this check in MediaStreamDevicesController::RequestPermissions but it seems like we should investigate why the request isn't being properly cancelled first. 

guidou: could you ptal?

Comment 3 by guidou@chromium.org, Feb 26 2018

I'll look into this.

Comment 4 by guidou@chromium.org, Feb 26 2018

Labels: Pri-1

Comment 5 by kenrb@chromium.org, Feb 27 2018

Labels: M-66 Security_Severity-Medium Security_Impact-Stable

Comment 6 by guidou@chromium.org, Feb 27 2018

The issue is that MediaStreamManager is sending a cancellation request using  NUM_MEDIA_TYPES as stream type, which gets ignored by PermissionBubbleMediaAccessHandler.

https://chromium-review.googlesource.com/c/chromium/src/+/939630 should fix it.

AFAICT, this has always been broken, dating back to when the code was first added in 2013.

Comment 7 by guidou@chromium.org, Feb 27 2018

Should we request merge to M65 once the fix for this lands?

Comment 8 by bugdroid1@chromium.org, Mar 1 2018

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/12c876ae82355de6285bf0879023f1d1f1822ecf

commit 12c876ae82355de6285bf0879023f1d1f1822ecf
Author: Guido Urdaneta <guidou@chromium.org>
Date: Thu Mar 01 10:47:37 2018

Fix MediaObserver notifications in MediaStreamManager.

This CL fixes the stream type used to notify MediaObserver about
cancelled MediaStream requests.

Before this CL, NUM_MEDIA_TYPES was used as stream type to indicate
that all stream types should be cancelled.
However, the MediaObserver end does not interpret NUM_MEDIA_TYPES this
way and the request to update the UI is ignored.

This CL sends a separate notification for each stream type so that the
UI actually gets updated for all stream types in use.

Bug:  816033 
Change-Id: Ib7d3b3046d1dd0976627f8ab38abf086eacc9405
Reviewed-on: https://chromium-review.googlesource.com/939630
Commit-Queue: Guido Urdaneta <guidou@chromium.org>
Reviewed-by: Raymes Khoury <raymes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540122}
[modify] https://crrev.com/12c876ae82355de6285bf0879023f1d1f1822ecf/content/browser/renderer_host/media/media_stream_manager.cc
[modify] https://crrev.com/12c876ae82355de6285bf0879023f1d1f1822ecf/content/browser/renderer_host/media/media_stream_manager_unittest.cc

Comment 9 by guidou@chromium.org, Mar 1 2018

Status: Fixed (was: Assigned)

Comment 10 by guidou@chromium.org, Mar 1 2018

Labels: reward-topanel

Comment 11 by sheriffbot@chromium.org, Mar 1 2018

Project Member
Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify

Comment 12 by awhalley@chromium.org, Mar 13 2018

Labels: -reward-topanel reward-unpaid reward-500
*** Boilerplate reminders! ***
Please do NOT publicly disclose details until a fix has been released to all our users. Early public disclosure may cancel the provisional reward. Also, please be considerate about disclosure when the bug affects a core library that may be used by other products. Please do NOT share this information with third parties who are not directly involved in fixing the bug. Doing so may cancel the provisional reward. Please be honest if you have already disclosed anything publicly or to third parties. Lastly, we understand that some of you are not interested in money. We offer the option to donate your reward to an eligible charity. If you prefer this option, let us know and we will also match your donation - subject to our discretion. Any rewards that are unclaimed after 12 months will be donated to a charity of our choosing.
*********************************

Comment 13 by awhalley@google.com, Mar 13 2018

Thanks! $500 for this one :-)

Comment 14 by chromium...@gmail.com, Mar 13 2018

Nice reward! Thanks Andrew as ever :-)

Comment 15 by sheriffbot@chromium.org, Mar 16 2018

Project Member
Labels: Merge-Request-66

Comment 16 by sheriffbot@chromium.org, Mar 16 2018

Project Member
Labels: -Merge-Request-66 Merge-Review-66 Hotlist-Merge-Review
This bug requires manual review: M66 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), josafat@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 17 by guidou@chromium.org, Mar 16 2018

Note that this fix landed originally in M66, so it does not need to be merged there.

Comment 18 by abdulsyed@google.com, Mar 19 2018

Labels: -Merge-Review-66 Merge-Approved-66
Merge approved 66. branch3359

Comment 19 by awhalley@google.com, Mar 19 2018

Labels: -reward-unpaid reward-inprocess

Comment 20 by guidou@chromium.org, Mar 20 2018

Labels: -Merge-Approved-66
This landed originally in M66, so no merge is necessary.

Comment 21 by awhalley@google.com, Apr 17 2018

Labels: Release-0-M66

Comment 22 by awhalley@chromium.org, Apr 25 2018

Labels: CVE-2018-6103

Comment 23 by awhalley@chromium.org, Apr 25 2018

Labels: CVE_description-missing

Comment 24 by sheriffbot@chromium.org, Jun 8 2018

Project Member
Labels: -Restrict-View-SecurityNotify allpublic
This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 25 by awhalley@chromium.org, Dec 4

Labels: -CVE_description-missing CVE_description-submitted

Sign in to add a comment