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

Issue 607483 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: May 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug-Security



Sign in to add a comment

Security: Universal XSS converting IDL array/sequence values

Project Member Reported by j...@opera.com, Apr 28 2016

Issue description

VULNERABILITY DETAILS
This is an issue similar to 605910, found as a follow-up to fixing that. In some cases, when an C++ array of objects is converted by toV8() with a window proxy object as the "creation context", a malicious script can override the actual creation context used, and gain access to other origins.

The underlying issue exists in M50, but I have not been able to get the exploit working. It's not unlikely that the issue can be exploited in M50 as well.

VERSION
Chrome Version: 51.0.2704.22 beta
Operating System: all

 
exploit.zip
973 bytes Download
Project Member

Comment 1 by ClusterFuzz, Apr 28 2016

Status: Assigned (was: Unconfirmed)
Project Member

Comment 2 by bugdroid1@chromium.org, Apr 28 2016

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

commit eb335fd1006cef3299221ebd14a8795679660e32
Author: jl <jl@opera.com>
Date: Thu Apr 28 17:04:58 2016

Use correct creation context when converting sequences to V8

The |creationContext| argument is often a reference to a window proxy
object, that may become incorrect to use if the frame is navigated and/or
detached during the loop that converts values.

BUG= 607483 

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

[modify] https://crrev.com/eb335fd1006cef3299221ebd14a8795679660e32/third_party/WebKit/Source/bindings/core/v8/ToV8.h

Comment 3 by rsesek@chromium.org, Apr 28 2016

Labels: Security_Severity-High Security_Impact-Stable M-50 OS-All Pri-1
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 29 2016

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

commit 650cd4ace843e976e28fa87fa40bd9c017d39f63
Author: jl <jl@opera.com>
Date: Fri Apr 29 10:08:27 2016

Use [[DefineOwnProperty]] when converting IDL array values

This means using v8::Object::CreateDataProperty() rather than Set(), and
is in line with how the conversion is defined in WebIDL. The incorrect use
of Set() is observable by scripts that define setters on Array.prototype
for the properties "0", "1", "2" and so on.

Also apply the same fix to conversion of Vector<std::pair<>> into object.

BUG= 607483 

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

[add] https://crrev.com/650cd4ace843e976e28fa87fa40bd9c017d39f63/third_party/WebKit/LayoutTests/fast/js/webidl-sequence-conversion.html
[modify] https://crrev.com/650cd4ace843e976e28fa87fa40bd9c017d39f63/third_party/WebKit/Source/bindings/core/v8/ToV8.h

Comment 5 by j...@opera.com, May 3 2016

Status: Fixed (was: Assigned)
Project Member

Comment 6 by ClusterFuzz, May 3 2016

Labels: -Restrict-View-SecurityTeam Merge-Triage M-51 Restrict-View-SecurityNotify
Adding Merge-Triage label for tracking purposes.

Once your fix had sufficient bake time (on canary, dev as appropriate), please nominate your fix for merge by adding the Merge-Requested label.

When your merge is approved by the release manager, please start merging with higher milestone label first. Make sure to re-request merge for every milestone in the label list. You can get branch information on omahaproxy.appspot.com.

- Your friendly ClusterFuzz
Labels: -Merge-Triage Merge-Request-51

Comment 8 by tin...@google.com, May 9 2016

Labels: -Merge-Request-51 Merge-Approved-51 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M51 (branch: 2704)
Project Member

Comment 9 by bugdroid1@chromium.org, May 10 2016

Labels: -merge-approved-51 merge-merged-2704
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/29352c3d107adf7fadecf6c5073fa492f1ece220

commit 29352c3d107adf7fadecf6c5073fa492f1ece220
Author: Jens Widell <jl@opera.com>
Date: Tue May 10 11:07:26 2016

Use correct creation context when converting sequences to V8

The |creationContext| argument is often a reference to a window proxy
object, that may become incorrect to use if the frame is navigated and/or
detached during the loop that converts values.

BUG= 607483 

Review-Url: https://codereview.chromium.org/1924073003
Cr-Commit-Position: refs/heads/master@{#390408}
(cherry picked from commit eb335fd1006cef3299221ebd14a8795679660e32)

Review URL: https://codereview.chromium.org/1965613003 .

Cr-Commit-Position: refs/branch-heads/2704@{#466}
Cr-Branched-From: 6e53600def8f60d8c632fadc70d7c1939ccea347-refs/heads/master@{#386251}

[modify] https://crrev.com/29352c3d107adf7fadecf6c5073fa492f1ece220/third_party/WebKit/Source/bindings/core/v8/ToV8.h

Project Member

Comment 10 by bugdroid1@chromium.org, May 10 2016

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

commit 010f3edb08426c16a4374dd13e25ff3538fb43dc
Author: Jens Widell <jl@opera.com>
Date: Tue May 10 11:11:53 2016

Use [[DefineOwnProperty]] when converting IDL array values

This means using v8::Object::CreateDataProperty() rather than Set(), and
is in line with how the conversion is defined in WebIDL. The incorrect use
of Set() is observable by scripts that define setters on Array.prototype
for the properties "0", "1", "2" and so on.

Also apply the same fix to conversion of Vector<std::pair<>> into object.

BUG= 607483 

Review-Url: https://codereview.chromium.org/1936433002
Cr-Commit-Position: refs/heads/master@{#390610}
(cherry picked from commit 650cd4ace843e976e28fa87fa40bd9c017d39f63)

Review URL: https://codereview.chromium.org/1961343003 .

Cr-Commit-Position: refs/branch-heads/2704@{#467}
Cr-Branched-From: 6e53600def8f60d8c632fadc70d7c1939ccea347-refs/heads/master@{#386251}

[add] https://crrev.com/010f3edb08426c16a4374dd13e25ff3538fb43dc/third_party/WebKit/LayoutTests/fast/js/webidl-sequence-conversion.html
[modify] https://crrev.com/010f3edb08426c16a4374dd13e25ff3538fb43dc/third_party/WebKit/Source/bindings/core/v8/ToV8.h

Labels: Release-0-M51
Labels: reward-ineligible
Project Member

Comment 13 by sheriffbot@chromium.org, Aug 9 2016

Labels: -Restrict-View-SecurityNotify
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
Project Member

Comment 14 by sheriffbot@chromium.org, Oct 1 2016

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
Project Member

Comment 15 by sheriffbot@chromium.org, Oct 2 2016

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
Labels: allpublic

Sign in to add a comment