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

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac , Fuchsia
Pri: 1
Type: Bug-Security



Sign in to add a comment

Security: V8: JIT: JSBuiltinReducer::ReduceObjectCreate fails to ensure that the prototype is "null"

Project Member Reported by lokihardt@google.com, Dec 13 2017

Issue description

I think this commit has introduced the bug.
https://chromium.googlesource.com/v8/v8/+/ff7063c7d5d8ad8eafcce3da59e65d7fe2b4f915%5E%21/#F2

According to the description, Object.create is supposed to be inlined only when the prototype given as the parameter is "null".

The following check has to guarantee it, but it can't guarantee it. Any receiver can get through the check, then Map::GetObjectCreateMap may transition the prototype, which may lead to type confusion.
  if (!prototype_const->IsNull(isolate()) && !prototype_const->IsJSReceiver()) {
    return NoChange();
  }
  instance_map = Map::GetObjectCreateMap(prototype_const);

PoC:
var object;
function opt() {
    opt['x'] = 1.1;
    try {
        Object.create(object);
    } catch (e) {
    }

    for (let i = 0; i < 1000000; i++) {

    }
}

opt();
object = opt;
opt();

This bug is subject to a 90 day disclosure deadline. After 90 days elapse
or a patch has been made broadly available, the bug report will become
visible to the public.

 
Cc: cbruni@chromium.org
Components: Blink>JavaScript
Cc: mstarzinger@chromium.org
+V8 Sheriff
Cc: bmeu...@chromium.org jarin@chromium.org
Components: -Blink>JavaScript Blink>JavaScript>Compiler
Project Member

Comment 4 by bugdroid1@chromium.org, Dec 14 2017

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

commit a90503d8a9526f9a3e3df4e24e68c8aea1c74f14
Author: Michael Starzinger <mstarzinger@chromium.org>
Date: Thu Dec 14 11:21:10 2017

[compiler] Prevent dependency invalidation in release mode.

This makes sure that breaking dependencies during compilation is also
caught properly in release mode (not only in debug mode). When this
happens the generated code would be invalid from the beginning and we
need to prevent using such code.

R=bmeurer@chromium.org
BUG= chromium:794394 , chromium:786723 

Change-Id: I76fd85786c16807389f69a9c44b9f893004b1c6f
Reviewed-on: https://chromium-review.googlesource.com/826635
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50105}
[modify] https://crrev.com/a90503d8a9526f9a3e3df4e24e68c8aea1c74f14/src/compiler.cc

Cc: -mstarzinger@chromium.org verwa...@chromium.org
Owner: mstarzinger@chromium.org
Status: Assigned (was: Unconfirmed)

Comment 6 by cthomp@chromium.org, Dec 14 2017

Labels: Security_Impact-Stable OS-Android OS-Chrome OS-Fuchsia OS-Linux OS-Mac OS-Windows
Project Member

Comment 7 by bugdroid1@chromium.org, Dec 15 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/4a7eec590c81a2f953ea5526d9c8731ac4042efb

commit 4a7eec590c81a2f953ea5526d9c8731ac4042efb
Author: Michael Starzinger <mstarzinger@chromium.org>
Date: Fri Dec 15 12:36:34 2017

[turbofan] Fix prototype mutation in Object.create lowering.

This makes sure the builtin lowering of Object.create doesn't invalidate
any previously taken dependencies. Aborting compilation after such cases
would lead to repeating optimization attempts without learning, hence we
disallow such situations.

R=verwaest@chromium.org
BUG= chromium:794394 , chromium:786723 

Change-Id: I6b6928cab19692bbbe3cd241ade862a2306eb0c7
Reviewed-on: https://chromium-review.googlesource.com/827066
Commit-Queue: Michael Starzinger <mstarzinger@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50128}
[modify] https://crrev.com/4a7eec590c81a2f953ea5526d9c8731ac4042efb/src/compiler/js-builtin-reducer.cc
[modify] https://crrev.com/4a7eec590c81a2f953ea5526d9c8731ac4042efb/src/objects.cc
[modify] https://crrev.com/4a7eec590c81a2f953ea5526d9c8731ac4042efb/src/objects/map.h
[add] https://crrev.com/4a7eec590c81a2f953ea5526d9c8731ac4042efb/test/mjsunit/regress/regress-crbug-786723.js

Status: Fixed (was: Assigned)
Project Member

Comment 9 by sheriffbot@chromium.org, Dec 15 2017

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

Comment 10 by bugdroid1@chromium.org, Jan 2 2018

Labels: merge-merged-6.4
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/422d8c525a4975e1490235199cdbd41cf04688f5

commit 422d8c525a4975e1490235199cdbd41cf04688f5
Author: Jaroslav Sevcik <jarin@chromium.org>
Date: Tue Jan 02 15:43:55 2018

Merged: [turbofan] Fix prototype mutation in Object.create lowering.

Revision: 4a7eec590c81a2f953ea5526d9c8731ac4042efb

BUG= chromium:786723 , chromium:794394 
LOG=N
NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true
TBR=hablich@chromium.org

Change-Id: I3677b90514a710e6dcffe27a346de6ec895859f6
Reviewed-on: https://chromium-review.googlesource.com/847483
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/branch-heads/6.4@{#24}
Cr-Branched-From: 0407506af3d9d7e2718be1d8759296165b218fcf-refs/heads/6.4.388@{#1}
Cr-Branched-From: a5fc4e085ee543cb608eb11034bc8f147ba388e1-refs/heads/master@{#49724}
[modify] https://crrev.com/422d8c525a4975e1490235199cdbd41cf04688f5/src/compiler/js-builtin-reducer.cc
[modify] https://crrev.com/422d8c525a4975e1490235199cdbd41cf04688f5/src/objects.cc
[modify] https://crrev.com/422d8c525a4975e1490235199cdbd41cf04688f5/src/objects/map.h
[add] https://crrev.com/422d8c525a4975e1490235199cdbd41cf04688f5/test/mjsunit/regress/regress-crbug-786723.js

Labels: Release-0-M64
Cc: pelizzi@google.com
Project Member

Comment 13 by sheriffbot@chromium.org, Mar 24

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