getDisplayMedia() shares entire screen—the least safe choice—by default, violating spec.
Reported by
wing...@gmail.com,
Jan 10
|
||||||
Issue descriptionUserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/73.0.3667.0 Safari/537.36 Steps to reproduce the problem: 1. Open https://jsfiddle.net/jib1/c91jxbrt/ 2. Click the "Share" button. What is the expected behavior? Click shouldn't work, because the "Share" button is disabled by default (e.g. because the safer "Application Window" tab is the default), and the end-user must actively pick something to share before the "Share" choice becomes available. What went wrong? My entire screen is shared immediately. This is the least safe option from a security perspective [1][2], and violates the "strongly advised" "elevated permission" section of the spec [3]. [1] https://blog.mozilla.org/webrtc/share-browser-windows-entire-screen-sites-trust/ [2] https://bugs.chromium.org/p/chromium/issues/detail?id=920733 [3] https://w3c.github.io/mediacapture-screen-share/#elevated-permissions Did this work before? Yes 72 (this risk is new from a security perspective with getDisplayMedia exposed to untrusted sites) Does this work in other browsers? Yes Chrome version: 73.0.3667.0 Channel: canary OS Version: OS X 10.13.6 Flash Version: Expected behavior is inspired by Firefox Nightly, and based on a weak interpretation of "elevated permission" as "at least not the default choice". Firefox additionally displays this warning whenever "elevated permission" sources are chosen by the user: "⚠ Only share screens/Firefox with sites you trust. Sharing can allow deceptive sites to browse as you and steal your private data. Learn more" ...where clicking "Learn more" opens https://support.mozilla.org/en-US/kb/screenshare-safety
,
Jan 10
,
Jan 11
,
Jan 11
,
Jan 11
Able to reproduce the issue on reported chrome reported version#73.0.3667.0 and on Beta# 72.0.3626.53, same issue is not seen on Stable# 71.0.3578.98 using Mac 10.14.0, Windows 10 and Ubuntu 17.10 hence providing Bisect Info. Bisect Info: ================ Good build: 72.0.3608.4 Bad build: 72.0.3610.0 You are probably looking for a change made after 607286 (known good), but no later than 607287 (first known bad). https://chromium.googlesource.com/chromium/src/+log/ea8b4707ffc2000756fd8bc2537c9994a9c2901f..8de9d5f05d03904d1c9868c765b1afef18ee5b5c Change-Id: I2d8cec26bae32281e50137dfd1616926b66e61a8 Reviewed-on: https://chromium-review.googlesource.com/c/1292439 @Emircan Uysaler: Please confirm the issue and help in re-assigning if it is not related to your change. Adding ReleaseBlock-Stable for M-72, feel free to remove it if not applicable. Thanks..!
,
Jan 12
Thanks for the report wingboy@gmail.com. I agree that having a pre-selected item isn't ideal. We do not have a pre-selected item in application and tab selection views, so I will check why that is the case for screen selection. It would be safer to require 2 user actions rather than 1. Note that we kept the UI consistent with the earlier extensions UI intentionally, and this might be a conscious decision coming from then. Still, we require user to explicitly go an click the "Share" button. Additionally, we display a warning text above the preview as we treat every single selection as an elevated permission. So, I think our implementation is still compliant with the spec. #5, this is not really a regression because it points to the CL where this feature is enabled. Before that there was no such functionality which is why there was no repro, so I am cleaning the flags to indicate.
,
Jan 12
qiangchen@, niklase@ pre-selection of window is done pretty explicitly. WDYT about removing it as suggested? https://chromium-review.googlesource.com/c/chromium/src/+/1407669
,
Jan 14
#7, Yeah I think that change is fine.
,
Jan 14
#8 thanks, I sent it for review.
,
Jan 17
(6 days ago)
Even with the change up for review here, the UX is still promoting the "scary" Entire Desktop source pretty heavily over the safer choices. That seems concerning, given that most users won't understand the different levels of security risks involved. I think most users would assume Google's default choice to be the most appropriate and safe for them. Why not make the "Application Window" tab the default? This would nudge users toward considering safer choices first, treating the Entire Desktop as a last resort. This would also even out access: 2 clicks for application window, and 2 clicks for Entire Desktop. Versus today's 3 clicks for application window, and 1 click for Entire Desktop (2 with patch ufr).
,
Jan 18
(4 days ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/df72eb18afdb13c6a6f42bc47da87a9c939347b7 commit df72eb18afdb13c6a6f42bc47da87a9c939347b7 Author: Emircan Uysaler <emircan@chromium.org> Date: Fri Jan 18 21:17:06 2019 Do not preselect screen view for getDisplayMedia() Bug: 920752 Change-Id: Ic4fcfb264b16425505185a409bdba583bb0e7459 Reviewed-on: https://chromium-review.googlesource.com/c/1407669 Commit-Queue: Emircan Uysaler <emircan@chromium.org> Reviewed-by: Avi Drissman <avi@chromium.org> Reviewed-by: Qiang Chen <qiangchen@chromium.org> Cr-Commit-Position: refs/heads/master@{#624284} [modify] https://crrev.com/df72eb18afdb13c6a6f42bc47da87a9c939347b7/chrome/browser/ui/cocoa/media_picker/desktop_media_picker_controller.mm [modify] https://crrev.com/df72eb18afdb13c6a6f42bc47da87a9c939347b7/chrome/browser/ui/cocoa/media_picker/desktop_media_picker_controller_unittest.mm [modify] https://crrev.com/df72eb18afdb13c6a6f42bc47da87a9c939347b7/chrome/browser/ui/views/desktop_capture/desktop_media_list_view.cc [modify] https://crrev.com/df72eb18afdb13c6a6f42bc47da87a9c939347b7/chrome/browser/ui/views/desktop_capture/desktop_media_picker_views_unittest.cc
,
Today
(13 hours ago)
Re #10, thanks for the input. We are currently discussing redesigning this UI(for both extensions and getDisplayMedia) and will bring up this feedback regarding the default section. Marking this bug as fixed as #11 landed. |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by phistuck@chromium.org
, Jan 10