New issue
Advanced search Search tips

Issue 852258 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

JSTypedArray ByteLength out of bounds

Reported by scdengy...@gmail.com, Jun 13 2018

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/67.0.3396.79 Safari/537.36

Steps to reproduce the problem:
poc.js
var typearr0 = new Int16Array(0x24924925);
typearr2 = typearr0.slice(7684);

What is the expected behavior?

What went wrong?
~/trees/v8/out.gn/ia32.debug.69123/d8 poc.js
1227133514
abort: CSA_ASSERT failed: IntPtrLessThanOrEqual(count_bytes, target_byte_length) [../../src/builtins/builtins-typed-array-gen.cc:1370]

==== JS stack trace =========================================

    0: ExitFrame [pc: 0x48c0608a]
Security context: 0x3619277d <JSObject>#0#
    1: slice [0x36187599](this=0x36198bdd <Int16Array map = 0x32285339>#1#,7684)
    2: /* anonymous */ [0x36195b51] [poc.js:3] [bytecode=0x36195aa5 offset=76](this=0x36186c5d <JSGlobal Object>#2#)
    3: InternalFrame [pc: 0x2f29399f]
    4: EntryFrame [pc: 0x2f28a6f1]

==== Details ================================================

[0]: ExitFrame [pc: 0x48c0608a]
[1]: slice [0x36187599](this=0x36198bdd <Int16Array map = 0x32285339>#1#,7684) {
// optimized frame
--------- s o u r c e   c o d e ---------
<No Source>
-----------------------------------------
}
[2]: /* anonymous */ [0x36195b51] [poc.js:3] [bytecode=0x36195aa5 offset=76](this=0x36186c5d <JSGlobal Object>#2#) {
  // stack-allocated locals
  var .result = 0x22a0438d <undefined>
  // expression stack (top to bottom)
  [05] : 7684
  [04] : 0x36198bdd <Int16Array map = 0x32285339>#1#
  [03] : 7684
  [02] : 0x36198bdd <Int16Array map = 0x32285339>#1#
  [01] : 0x36187599 <JSFunction slice (sfi = 0x3b893bd5)>#3#
--------- s o u r c e   c o d e ---------
var typearr0 = new Int16Array(0x24924925);\x0aprint(typearr0.byteLength)\x0atypearr2 = typearr0.slice(7684);\x0aprint(typearr2.byteLength)\x0a
-----------------------------------------
}

[3]: InternalFrame [pc: 0x2f29399f]
[4]: EntryFrame [pc: 0x2f28a6f1]
==== Key         ============================================

 #0# 0x3619277d: 0x3619277d <JSObject>
 #1# 0x36198bdd: 0x36198bdd <Int16Array map = 0x32285339>
 #2# 0x36186c5d: 0x36186c5d <JSGlobal Object>
 #3# 0x36187599: 0x36187599 <JSFunction slice (sfi = 0x3b893bd5)>
=====================

Received signal 4 ILL_ILLOPN 0000f514c156

==== C stack trace ===============================

 [0x0000f5150096]
 [0x0000f514ff9d]
 [0x0000f7787bd0]
 [0x0000f514c156]
 [0x0000f6dcee6f]
 [0x0000f6dce6a9]
 [0x000048c0608a]
 [0x00004af12775]
 [0x00002f29b280]
 [0x00002f29399f]
 [0x00002f28a6f1]
 [0x0000f6580a96]
 [0x0000f657db30]
 [0x0000f657cf70]
 [0x0000f657cd67]
 [0x0000f59f7008]
 [0x000056633c0c]
 [0x00005664982c]
 [0x00005664dfb6]
 [0x00005665020e]
 [0x0000566507a2]
 [0x0000f4cef637]
[end of stack trace]
[1]    108227 illegal hardware instruction (core dumped)  ~/trees/v8/out.gn/ia32.debug.69123/d8 poc.js

Did this work before? N/A 

Chrome version: 67.0.3396.79  Channel: n/a
OS Version: OS X 10.13.5
Flash Version:
 
Project Member

Comment 1 by ClusterFuzz, Jun 13 2018

ClusterFuzz is analyzing your testcase. Developers can follow the progress at https://clusterfuzz.com/testcase?key=5894153108717568.

Comment 2 by palmer@chromium.org, Jun 13 2018

Cc: titzer@chromium.org rmcilroy@chromium.org
Components: Blink>JavaScript
Labels: OS-Android OS-Chrome OS-Fuchsia OS-Linux OS-Windows
Summary: JSTypedArray ByteLength out of bounds (was: JSTypedArray ByteLength out of bound)
Cc: jgruber@chromium.org
Owner: petermarshall@chromium.org
Status: Assigned (was: Unconfirmed)
Status: Started (was: Assigned)
I think the byte length is actually correct, but we try to load it as a Smi when it can actually be a HeapNumber. We only do this in the debug code though - the other calculations are correct as far as I can tell.

Comment 6 by titzer@chromium.org, Jun 14 2018

We should store the byte length of a typed array as an unboxed 32-bit number. We have a longer term goal of allowing full 4gb WebAssembly memories, and with SMIs becoming 31 bits on 64-bit due to pointer compression in the near future, we should probably do this migration soon.
We can already exceed an unsigned 32 bit value in byte length though - max byte length is 8 (size of float64 elements) * maxSmi, which is roughly 2 ^ 34 on 64 bit, and 2 ^ 33 on 32 bit - although allocation of the buffer would clearly fail on 32 bit.

Max SMI shrinking will halve the maximum typed array size - so we should look into allowing Number-lengthed typed array soon.

Comment 8 by titzer@chromium.org, Jun 14 2018

How about storing the byte length in the array buffer object as an unboxed uintptr_t (i.e. 64 bit on 64 bit platforms)? We already store the base address this way. Eliminating the accessor which returns a tagged value would force us to fix all SMI uses. Wdyt?

Comment 9 by wfh@chromium.org, Jun 14 2018

Labels: Security_Severity-High Security_Impact-Stable
Project Member

Comment 10 by bugdroid1@chromium.org, Jun 15 2018

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

commit d69df91c27018affdba41f89abea59ff4887c30b
Author: Peter Marshall <petermarshall@chromium.org>
Date: Fri Jun 15 08:26:41 2018

[typedarray] Fix incorrect access to typed array byte offset.

Byte offset can be outside of Smi range and must be loaded as a Number
rather than a Smi.

Bug:  chromium:852258 
Change-Id: Ida6e07ba68a050d4f5a9f28500986cc67c619b4c
Reviewed-on: https://chromium-review.googlesource.com/1100886
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#53748}
[modify] https://crrev.com/d69df91c27018affdba41f89abea59ff4887c30b/src/builtins/builtins-typed-array-gen.cc
[add] https://crrev.com/d69df91c27018affdba41f89abea59ff4887c30b/test/mjsunit/regress/regress-852258.js

Project Member

Comment 11 by sheriffbot@chromium.org, Jun 15 2018

Labels: Target-67 M-67
Project Member

Comment 12 by sheriffbot@chromium.org, Jun 15 2018

Labels: -Pri-2 Pri-1
Labels: -Security_Impact-Stable -Security_Severity-High Security_Severity-Low Security_Impact-None
Status: Fixed (was: Started)
This is fixed now. This is not an exploitable security issue - the ByteLength doesn't actually go out of bounds, we just read it incorrectly, but only in debug code for a sanity-check, so this has no impact on released versions.
Project Member

Comment 14 by sheriffbot@chromium.org, Jun 18 2018

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Labels: Hotlist-Torque
Cc: tebbi@chromium.org
Cc: jarin@chromium.org
Project Member

Comment 18 by sheriffbot@chromium.org, Sep 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
Labels: reward-topanel
Labels: -reward-topanel reward-0

Sign in to add a comment