Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2694783002:
[builtins] treat all elements of detached TypedArray as undefined in A.p.includes (Closed)
Description[builtins] treat all elements of detached TypedArray as undefined in A.p.includes
If a TypedArray's buffer is neutered during elements processing, do not access actual elements
(which is a UAF and potential security issue), but instead treat each element as undefined,
and return true if searching for undefined. This seems to match the behaviour of v8 prior to
porting A.p.includes to C++
BUG=chromium:691323
R=cbruni@chromium.org, jkummerow@chromium.org, littledan@chromium.org
Patch Set 1 #Patch Set 2 : fix a typo #Patch Set 3 : Don't throw, but treat all elements as undefined instead, the way it used to work #Patch Set 4 : also respect startFrom vs length #Messages
Total messages: 25 (19 generated)
The CQ bit was checked by caitp@igalia.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
PTAL
The CQ bit was checked by caitp@igalia.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== [builtins] throw TypeError if typedarray buffer is detached in A.p.includes Verify buffer is not neutered during TypedArray impl in elements.cc for ElementsAccessor::IncludesValue(), preventing a UAF bug. BUG=chromium:691323 R=cbruni@chromium.org ========== to ========== [builtins] throw TypeError if typedarray buffer is detached in A.p.includes Verify buffer is not neutered during TypedArray impl in elements.cc for ElementsAccessor::IncludesValue(), preventing a UAF bug. BUG=chromium:691323 R=cbruni@chromium.org, jkummerow@chromium.org ==========
caitp@igalia.com changed reviewers: + jkummerow@chromium.org
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_linux_arm64_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel_ng/build...)
Description was changed from ========== [builtins] throw TypeError if typedarray buffer is detached in A.p.includes Verify buffer is not neutered during TypedArray impl in elements.cc for ElementsAccessor::IncludesValue(), preventing a UAF bug. BUG=chromium:691323 R=cbruni@chromium.org, jkummerow@chromium.org ========== to ========== [builtins] treat all elements of detached TypedArray as undefined in A.p.includes If a TypedArray's buffer is neutered during elements processing, do not access actual elements (which is a UAF and potential security issue), but instead treat the entire array as undefined, and return true if searching for undefined. This seems to match the behaviour of v8 prior to porting A.p.includes to C++ BUG=chromium:691323 R=cbruni@chromium.org, jkummerow@chromium.org ==========
Description was changed from ========== [builtins] treat all elements of detached TypedArray as undefined in A.p.includes If a TypedArray's buffer is neutered during elements processing, do not access actual elements (which is a UAF and potential security issue), but instead treat the entire array as undefined, and return true if searching for undefined. This seems to match the behaviour of v8 prior to porting A.p.includes to C++ BUG=chromium:691323 R=cbruni@chromium.org, jkummerow@chromium.org ========== to ========== [builtins] treat all elements of detached TypedArray as undefined in A.p.includes If a TypedArray's buffer is neutered during elements processing, do not access actual elements (which is a UAF and potential security issue), but instead treat the entire array as undefined, and return true if searching for undefined. This seems to match the behaviour of v8 prior to porting A.p.includes to C++ BUG=chromium:691323 R=cbruni@chromium.org, jkummerow@chromium.org, littledan@chromium.org ==========
caitp@igalia.com changed reviewers: + littledan@chromium.org
On 2017/02/13 15:55:00, caitp wrote: > PTAL Per discussion offline (and on another similar CL), it seems that web reality is to treat elements of detached TypedArrays as undefined, so this has changed to do that rather than throwing.
lgtm
The CQ bit was checked by caitp@igalia.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== [builtins] treat all elements of detached TypedArray as undefined in A.p.includes If a TypedArray's buffer is neutered during elements processing, do not access actual elements (which is a UAF and potential security issue), but instead treat the entire array as undefined, and return true if searching for undefined. This seems to match the behaviour of v8 prior to porting A.p.includes to C++ BUG=chromium:691323 R=cbruni@chromium.org, jkummerow@chromium.org, littledan@chromium.org ========== to ========== [builtins] treat all elements of detached TypedArray as undefined in A.p.includes If a TypedArray's buffer is neutered during elements processing, do not access actual elements (which is a UAF and potential security issue), but instead treat the entire array as undefined, and return true if searching for undefined. This seems to match the behaviour of v8 prior to porting A.p.includes to C++ BUG=chromium:691323 R=cbruni@chromium.org, jkummerow@chromium.org, littledan@chromium.org ==========
Description was changed from ========== [builtins] treat all elements of detached TypedArray as undefined in A.p.includes If a TypedArray's buffer is neutered during elements processing, do not access actual elements (which is a UAF and potential security issue), but instead treat the entire array as undefined, and return true if searching for undefined. This seems to match the behaviour of v8 prior to porting A.p.includes to C++ BUG=chromium:691323 R=cbruni@chromium.org, jkummerow@chromium.org, littledan@chromium.org ========== to ========== [builtins] treat all elements of detached TypedArray as undefined in A.p.includes If a TypedArray's buffer is neutered during elements processing, do not access actual elements (which is a UAF and potential security issue), but instead treat each element as undefined, and return true if searching for undefined. This seems to match the behaviour of v8 prior to porting A.p.includes to C++ BUG=chromium:691323 R=cbruni@chromium.org, jkummerow@chromium.org, littledan@chromium.org ==========
The CQ bit was checked by caitp@igalia.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_linux64_verify_csa_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_verify_csa_rel_n...) v8_linux64_verify_csa_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_verify_csa_rel_n...)
lgtm
On 2017/02/15 13:37:10, Camillo Bruni wrote: > lgtm I think you should land your version, the WasNeutered helper is cleaner than this, + I believe yours isn't failing tests the due to the DisallowHeapAllocation thing like this one is
On 2017/02/15 at 13:39:33, caitp wrote: > On 2017/02/15 13:37:10, Camillo Bruni wrote: > > lgtm > > I think you should land your version, the WasNeutered helper is cleaner than this, + I believe yours isn't failing tests the due to the DisallowHeapAllocation thing like this one is ok, will fix mine |
