New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 688569 link

Starred by 1 user
Status: Fixed
Owner:
█ sigbjo...@opera.com
Email to this user bounced
Closed: Feb 2017
Cc:
jochen@chromium.org
█ mkwst@chromium.org
rbyers@chromium.org
█ sigbjo...@opera.com
esprehn@chromium.org
dcheng@chromium.org
lanwei@chromium.org
lfg@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug-Security



Sign in to add a comment

Security: Fix all ScriptWrappables stored in a static Persistent

Project Member Reported by dcheng@chromium.org, Feb 3 2017 
It's tempting to do this for objects with no internal state, but doing so is unsafe, as it leaks wrappers cross-origin and (likely) leads to UXSS.

 Issue 687844  fixed this for window.external, but there are others. Per https://bugs.chromium.org/p/chromium/issues/detail?id=687844#c15, the InputDeviceCapabilities also have this problem.

In a followup, we should make sure that we static_assert that this doesn't happen.

Comment 1 by sigbjo...@opera.com, Feb 4 2017 

https://codereview.chromium.org/2674973005/ for the static_assert(), but it cannot go ahead until https://codereview.chromium.org/2675793005/ (in whatever form it ends up as) has landed.

Comment 2 by sigbjo...@opera.com, Feb 6 2017

Cc: lanwei@chromium.org rbyers@chromium.org
Project Member

Comment 3 by bugdroid1@chromium.org, Feb 7 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3867c9867bbb9bbe1d1d4c2c68b60073cb45caaf

commit 3867c9867bbb9bbe1d1d4c2c68b60073cb45caaf
Author: sigbjornf <sigbjornf@opera.com>
Date: Tue Feb 07 11:11:55 2017

Track constant InputDeviceCapabilities objects per-window.

Do not keep global main thread Persistent<>s for the two
constant InputDeviceCapabilities objects, as that will
end up sharing wrapper objects across contexts.

R=dcheng
BUG= 688569 

Review-Url: https://codereview.chromium.org/2675793005
Cr-Commit-Position: refs/heads/master@{#448597}

[add] https://crrev.com/3867c9867bbb9bbe1d1d4c2c68b60073cb45caaf/third_party/WebKit/LayoutTests/http/tests/security/cross-frame-mouse-source-capabilities.html
[add] https://crrev.com/3867c9867bbb9bbe1d1d4c2c68b60073cb45caaf/third_party/WebKit/LayoutTests/http/tests/security/resources/cross-frame-mouse-source-capabilities.html
[modify] https://crrev.com/3867c9867bbb9bbe1d1d4c2c68b60073cb45caaf/third_party/WebKit/Source/core/events/CompositionEvent.cpp
[modify] https://crrev.com/3867c9867bbb9bbe1d1d4c2c68b60073cb45caaf/third_party/WebKit/Source/core/events/KeyboardEvent.cpp
[modify] https://crrev.com/3867c9867bbb9bbe1d1d4c2c68b60073cb45caaf/third_party/WebKit/Source/core/events/MouseEvent.cpp
[modify] https://crrev.com/3867c9867bbb9bbe1d1d4c2c68b60073cb45caaf/third_party/WebKit/Source/core/events/TouchEvent.cpp
[modify] https://crrev.com/3867c9867bbb9bbe1d1d4c2c68b60073cb45caaf/third_party/WebKit/Source/core/events/UIEvent.cpp
[modify] https://crrev.com/3867c9867bbb9bbe1d1d4c2c68b60073cb45caaf/third_party/WebKit/Source/core/events/UIEvent.h
[modify] https://crrev.com/3867c9867bbb9bbe1d1d4c2c68b60073cb45caaf/third_party/WebKit/Source/core/frame/DOMWindow.cpp
[modify] https://crrev.com/3867c9867bbb9bbe1d1d4c2c68b60073cb45caaf/third_party/WebKit/Source/core/frame/DOMWindow.h
[modify] https://crrev.com/3867c9867bbb9bbe1d1d4c2c68b60073cb45caaf/third_party/WebKit/Source/core/html/HTMLSelectElement.cpp
[modify] https://crrev.com/3867c9867bbb9bbe1d1d4c2c68b60073cb45caaf/third_party/WebKit/Source/core/input/EventHandler.cpp
[modify] https://crrev.com/3867c9867bbb9bbe1d1d4c2c68b60073cb45caaf/third_party/WebKit/Source/core/input/GestureManager.cpp
[modify] https://crrev.com/3867c9867bbb9bbe1d1d4c2c68b60073cb45caaf/third_party/WebKit/Source/core/input/InputDeviceCapabilities.cpp
[modify] https://crrev.com/3867c9867bbb9bbe1d1d4c2c68b60073cb45caaf/third_party/WebKit/Source/core/input/InputDeviceCapabilities.h
[modify] https://crrev.com/3867c9867bbb9bbe1d1d4c2c68b60073cb45caaf/third_party/WebKit/Source/core/input/KeyboardEventManager.cpp

Comment 4 by sigbjo...@opera.com, Feb 7 2017 

(Will let #3 bake in, but I can request merges after that.)

Comment 5 by sigbjo...@opera.com, Feb 7 2017

Owner: sigbjo...@opera.com
Status: Started (was: Available)

Comment 6 by jialiul@chromium.org, Feb 7 2017

Labels: -M-55 Security_Severity-High M-57 Security_Impact-Stable

Comment 7 by sigbjo...@opera.com, Feb 8 2017

Labels: Merge-Request-57
Project Member

Comment 8 by sheriffbot@chromium.org, Feb 8 2017

Labels: -Merge-Request-57 Hotlist-Merge-Approved Merge-Approved-57
Your change meets the bar and is auto-approved for M57. Please go ahead and merge the CL to branch 2987 manually. Please contact milestone owner if you have questions.
Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop)

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

Comment 9 by gov...@chromium.org, Feb 9 2017 

Please merge your change to M57 branch 2987 before 5:00 PM PT, Friday 02/10 (sooner the better please) so we can take it in for next week beta release. Thank you.
Project Member

Comment 10 by sheriffbot@chromium.org, Feb 9 2017

Status: Fixed (was: Started)
Please mark security bugs as fixed as soon as the fix lands, and before requesting merges. This update is based on the merge- labels applied to this issue. Please reopen if this update was incorrect.

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

Comment 11 by bugdroid1@chromium.org, Feb 9 2017

Labels: -merge-approved-57 merge-merged-2987
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/aca3d111188556531c1f536bc6aff38f2904b059

commit aca3d111188556531c1f536bc6aff38f2904b059
Author: Sigbjorn Finne <sigbjornf@opera.com>
Date: Thu Feb 09 14:54:51 2017

Track constant InputDeviceCapabilities objects per-window.

Do not keep global main thread Persistent<>s for the two
constant InputDeviceCapabilities objects, as that will
end up sharing wrapper objects across contexts.

R=dcheng
BUG= 688569 

Review-Url: https://codereview.chromium.org/2675793005
Cr-Commit-Position: refs/heads/master@{#448597}
(cherry picked from commit 3867c9867bbb9bbe1d1d4c2c68b60073cb45caaf)

Conflicts:
	third_party/WebKit/Source/core/events/MouseEvent.cpp
	third_party/WebKit/Source/core/events/TouchEvent.cpp
	third_party/WebKit/Source/core/frame/DOMWindow.cpp
	third_party/WebKit/Source/core/frame/DOMWindow.h
	third_party/WebKit/Source/core/input/EventHandler.cpp

Review-Url: https://codereview.chromium.org/2687903002 .
Cr-Commit-Position: refs/branch-heads/2987@{#405}
Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943}

[add] https://crrev.com/aca3d111188556531c1f536bc6aff38f2904b059/third_party/WebKit/LayoutTests/http/tests/security/cross-frame-mouse-source-capabilities.html
[add] https://crrev.com/aca3d111188556531c1f536bc6aff38f2904b059/third_party/WebKit/LayoutTests/http/tests/security/resources/cross-frame-mouse-source-capabilities.html
[modify] https://crrev.com/aca3d111188556531c1f536bc6aff38f2904b059/third_party/WebKit/Source/core/events/CompositionEvent.cpp
[modify] https://crrev.com/aca3d111188556531c1f536bc6aff38f2904b059/third_party/WebKit/Source/core/events/KeyboardEvent.cpp
[modify] https://crrev.com/aca3d111188556531c1f536bc6aff38f2904b059/third_party/WebKit/Source/core/events/MouseEvent.cpp
[modify] https://crrev.com/aca3d111188556531c1f536bc6aff38f2904b059/third_party/WebKit/Source/core/events/TouchEvent.cpp
[modify] https://crrev.com/aca3d111188556531c1f536bc6aff38f2904b059/third_party/WebKit/Source/core/events/UIEvent.cpp
[modify] https://crrev.com/aca3d111188556531c1f536bc6aff38f2904b059/third_party/WebKit/Source/core/events/UIEvent.h
[modify] https://crrev.com/aca3d111188556531c1f536bc6aff38f2904b059/third_party/WebKit/Source/core/frame/DOMWindow.cpp
[modify] https://crrev.com/aca3d111188556531c1f536bc6aff38f2904b059/third_party/WebKit/Source/core/frame/DOMWindow.h
[modify] https://crrev.com/aca3d111188556531c1f536bc6aff38f2904b059/third_party/WebKit/Source/core/html/HTMLSelectElement.cpp
[modify] https://crrev.com/aca3d111188556531c1f536bc6aff38f2904b059/third_party/WebKit/Source/core/input/EventHandler.cpp
[modify] https://crrev.com/aca3d111188556531c1f536bc6aff38f2904b059/third_party/WebKit/Source/core/input/GestureManager.cpp
[modify] https://crrev.com/aca3d111188556531c1f536bc6aff38f2904b059/third_party/WebKit/Source/core/input/InputDeviceCapabilities.cpp
[modify] https://crrev.com/aca3d111188556531c1f536bc6aff38f2904b059/third_party/WebKit/Source/core/input/InputDeviceCapabilities.h
[modify] https://crrev.com/aca3d111188556531c1f536bc6aff38f2904b059/third_party/WebKit/Source/core/input/KeyboardEventManager.cpp
Project Member

Comment 12 by sheriffbot@chromium.org, Feb 10 2017

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify

Comment 14 by awhalley@chromium.org, Feb 14 2017

Labels: -Hotlist-Merge-Approved

Comment 15 by awhalley@chromium.org, Mar 6 2017

Labels: Release-0-57

Comment 16 by awhalley@chromium.org, Mar 6 2017

Labels: -Release-0-57 Release-0-M57
Project Member

Comment 17 by sheriffbot@chromium.org, May 19 2017

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
►

Sign in to add a comment