Overview

High
Severity
CVSS
No
Exploited ITW
Fixed
Fix Status
ImpactOut of bounds memory access in V8
DescriptionOut of bounds memory access in V8
ComponentV8
Bug ClassOOB
Tracker374627491
Fix commit5c3b50c26c50 (v8/v8) +39/-6
CISA KEVNot listed
Creditedddme
Disclosed2025-01-14

Changed Functions

FunctionChangeNotes
if
src/interpreter/bytecode-generator.cc
modified
B
test/mjsunit/regress/regress-374627491.js
modified
C
test/mjsunit/regress/regress-374627491.js
modified
constructor
test/mjsunit/regress/regress-374627491.js
modified
switch
test/mjsunit/regress/regress-374627491.js
modified

Files Changed

  • src/interpreter/bytecode-generator.cc
  • test/mjsunit/regress/regress-374627491.js
From 5c3b50c26c50e68dbedf8ff991249e75e46ef06e Mon Sep 17 00:00:00 2001
From: Shu-yu Guo <syg@chromium.org>
Date: Mon, 18 Nov 2024 16:02:28 -0800
Subject: [PATCH] [interpreter] Fix hole elision scope for switch jump tables

Fixed: 374627491
Change-Id: I7b6142b45295ba795a8ae8a90692fc09e704b65d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6032225
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Shu-yu Guo <syg@chromium.org>
Cr-Commit-Position: refs/heads/main@{#97274}
---

diff --git a/src/interpreter/bytecode-generator.cc b/src/interpreter/bytecode-generator.cc
index 7912eba..9e22967 100644
--- a/src/interpreter/bytecode-generator.cc
+++ b/src/interpreter/bytecode-generator.cc
@@ -2526,6 +2526,9 @@
   // Are we still using any if-else bytecodes to evaluate the switch?
   bool use_jumps = n_comp_cases != 0;
 
+  // Does the comparison for non-jump table jumps need an elision scope?
+  bool jump_comparison_needs_hole_check_elision_scope = false;
+
   SwitchBuilder switch_builder(builder(), block_coverage_builder_, stmt,
                                n_comp_cases, jump_table);
   ControlScopeForBreakable scope(this, stmt, &switch_builder);
@@ -2583,6 +2586,10 @@
                                          info.covered_cases);
 
     if (use_jumps) {
+      // When using a jump table, the first jump comparison is conditionally
+      // executed if the discriminant wasn't matched by anything in the jump
+      // table, and so needs its own elision scope.
+      jump_comparison_needs_hole_check_elision_scope = true;
       builder()->LoadAccumulatorWithRegister(r1);
     }
   }
@@ -2603,16 +2610,14 @@
       // The comparisons linearly dominate, so no need to open a new elision
       // scope for each one.
       std::optional<HoleCheckElisionScope> elider;
-      bool first_jump_emitted = false;
       for (int i = 0; i < clauses->length(); ++i) {
         CaseClause* clause = clauses->at(i);
         if (clause->is_default()) {
           info.default_case = i;
         } else if (!info.CaseExists(clause->label())) {
-          // The first non-default label is
-          // unconditionally executed, so we only need to emplace it before
-          // visiting the second non-default label.
-          if (first_jump_emitted) elider.emplace(this);
+          if (jump_comparison_needs_hole_check_elision_scope && !elider) {
+            elider.emplace(this);
+          }
 
           // Perform label comparison as if via '===' with tag.
           VisitForAccumulatorValue(clause->label());
@@ -2623,7 +2628,9 @@
 #endif
           switch_builder.JumpToCaseIfTrue(ToBooleanMode::kAlreadyBoolean,
                                           case_compare_ctr++);
-          first_jump_emitted = true;
+          // The second and subsequent non-default comparisons are always
+          // conditionally executed, and need an elision scope.
+          jump_comparison_needs_hole_check_elision_scope = true;
         }
       }
     }
diff --git a/test/mjsunit/regress/regress-374627491.js b/test/mjsunit/regress/regress-374627491.js
new file mode 100644
index 0000000..ebb7e1d
--- /dev/null
+++ b/test/mjsunit/regress/regress-374627491.js
@@ -0,0 +1,26 @@
+// Copyright 2024 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+class B { }
+class C extends B {
+  constructor() {
+    let x = 0;
+    switch (0) {
+      case 0:
+      case 1:
+      case 2:
+      case 3:
+      case 4:
+      case 5:
+      case 6:
+      case 7:
+      case 8:
+      case 9:
+        x += this;
+        break;
+      case this:
+    }
+  }
+}
+assertThrows(() => { new C(); }, ReferenceError);
Loading diff…

Regression Test / PoC

shipped with the fix
diff --git a/test/mjsunit/regress/regress-374627491.js b/test/mjsunit/regress/regress-374627491.js
new file mode 100644
index 0000000..ebb7e1d
--- /dev/null
+++ b/test/mjsunit/regress/regress-374627491.js
@@ -0,0 +1,26 @@
+// Copyright 2024 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+class B { }
+class C extends B {
+  constructor() {
+    let x = 0;
+    switch (0) {
+      case 0:
+      case 1:
+      case 2:
+      case 3:
+      case 4:
+      case 5:
+      case 6:
+      case 7:
+      case 8:
+      case 9:
+        x += this;
+        break;
+      case this:
+    }
+  }
+}
+assertThrows(() => { new C(); }, ReferenceError);
Loading diff…

Original Bug Report

reported by dd...@gmail.com

DCHECK failure in Handle<To> v8::internal::Cast(Handle<From>, const v8::SourceLocation &) [To = v8

Steps to reproduce the problem

  1. download d8-asan-linux-debug-v8-component-96641
  2. unzip that
  3. run d8 ./poc.js

Problem Description

#
# Fatal error in ../../src/objects/objects.cc, line 353
# Debug check failed: Handle<To> v8::internal::Cast(Handle<From>, const v8::SourceLocation &) [To = v8::internal::JSReceiver, From = v8::internal::Object].
#
#
#
#FailureMessage Object: 0x7bc62e2f9c60
==== C stack trace ===============================

    ./google-cloud-sdk/d8-asan-linux-debug-v8-component-96641/d8(__interceptor_backtrace+0x46) [0x55cbd91b17a6]
    /root/google-cloud-sdk/d8-asan-linux-debug-v8-component-96641/libv8_libbase.so(v8::base::debug::StackTrace::StackTrace()+0x13) [0x7fc6317c8573]
    /root/google-cloud-sdk/d8-asan-linux-debug-v8-component-96641/libv8_libplatform.so(+0x3687a) [0x7fc63171c87a]
    /root/google-cloud-sdk/d8-asan-linux-debug-v8-component-96641/libv8_libbase.so(V8_Fatal(char const*, int, char const*, ...)+0x2a0) [0x7fc631792c80]
    /root/google-cloud-sdk/d8-asan-linux-debug-v8-component-96641/libv8_libbase.so(+0x56d3f) [0x7fc631791d3f]
    /root/google-cloud-sdk/d8-asan-linux-debug-v8-component-96641/libv8.so(v8::internal::Handle<v8::internal::JSReceiver> v8::internal::Cast<v8::internal::JSReceiver, v8::internal::Object>(v8::internal::Handle<v8::internal::Object>, v8::SourceLocation const&)+0x1b2) [0x7fc635ba53f2]
    /root/google-cloud-sdk/d8-asan-linux-debug-v8-component-96641/libv8.so(v8::internal::Object::ConvertToNumeric(v8::internal::Isolate*, v8::internal::Handle<v8::internal::Object>)+0x1ba) [0x7fc637bd9dba]
    /root/google-cloud-sdk/d8-asan-linux-debug-v8-component-96641/libv8.so(v8::internal::Object::ToNumeric(v8::internal::Isolate*, v8::internal::Handle<v8::internal::Object>)+0x3d1) [0x7fc635aacda1]
    /root/google-cloud-sdk/d8-asan-linux-debug-v8-component-96641/libv8.so(+0x6acfa0d) [0x7fc6382b3a0d]
    /root/google-cloud-sdk/d8-asan-linux-debug-v8-component-96641/libv8.so(v8::internal::Runtime_ToNumeric(int, unsigned long*, v8::internal::Isolate*)+0x1da) [0x7fc6382b315a]
    /root/google-cloud-sdk/d8-asan-linux-debug-v8-component-96641/libv8.so(+0x3a58afd) [0x7fc63523cafd]
AddressSanitizer:DEADLYSIGNAL
=================================================================
==1914346==ERROR: AddressSanitizer: TRAP on unknown address 0x000000000000 (pc 0x7fc6317c3690 bp 0x7ffe27e76810 sp 0x7ffe27e76810 T0)
SCARINESS: 10 (signal)
    #0 0x7fc6317c3690 in v8::base::OS::Abort() src/base/platform/platform-posix.cc:730:7
    #1 0x7fc631792c9b in V8_Fatal(char const*, int, char const*, ...) src/base/logging.cc:215:3
    #2 0x7fc631791d3e in v8::base::(anonymous namespace)::DefaultDcheckHandler(char const*, int, char const*) src/base/logging.cc:59:3
    #3 0x7fc635ba53f1 in v8::internal::Handle<v8::internal::JSReceiver> v8::internal::Cast<v8::internal::JSReceiver, v8::internal::Object>(v8::internal::Handle<v8::internal::Object>, v8::SourceLocation const&) src/handles/handles-inl.h:50:3
    #4 0x7fc637bd9db9 in v8::internal::Object::ConvertToNumeric(v8::internal::Isolate*, v8::internal::Handle<v8::internal::Object>) src/objects/objects.cc:350:5
    #5 0x7fc635aacda0 in v8::internal::Object::ToNumeric(v8::internal::Isolate*, v8::internal::Handle<v8::internal::Object>) src/objects/objects-inl.h:816:10
    #6 0x7fc6382b3a0c in v8::internal::__RT_impl_Runtime_ToNumeric(v8::internal::Arguments<(v8::internal::ArgumentsType)0>, v8::internal::Isolate*) src/runtime/runtime-object.cc:1231:3
    #7 0x7fc6382b3159 in v8::internal::Runtime_ToNumeric(int, unsigned long*, v8::internal::Isolate*) src/runtime/runtime-object.cc:1227:1
    #8 0x7fc63523cafc in Builtins_CEntry_Return1_ArgvOnStack_NoBuiltinExit setup-isolate-deserialize.cc
    #9 0x7fc635454701 in Builtins_NonNumberToNumeric setup-isolate-deserialize.cc
    #10 0x7fc6358490d4 in Builtins_ShiftRightLogicalHandler setup-isolate-deserialize.cc
    #11 0x7fc634e5754e in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #12 0x7fc634e58162 in Builtins_InterpreterPushArgsThenFastConstructFunction setup-isolate-deserialize.cc
    #13 0x7fc63587725c in Builtins_ConstructHandler setup-isolate-deserialize.cc
    #14 0x7fc634e5754e in Builtins_InterpreterEntryTrampoline setup-isolate-deserialize.cc
    #15 0x7fc634e4e5db in Builtins_JSEntryTrampoline setup-isolate-deserialize.cc
    #16 0x7fc634e4e31e in Builtins_JSEntry setup-isolate-deserialize.cc
    #17 0x7fc63659536a in v8::internal::(anonymous namespace)::Invoke(v8::internal::Isolate*, v8::internal::(anonymous namespace)::InvokeParams const&) src/execution/simulator.h:191:12
    #18 0x7fc63659903c in v8::internal::Execution::CallScript(v8::internal::Isolate*, v8::internal::Handle<v8::internal::JSFunction>, v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>) src/execution/execution.cc:517:10
    #19 0x7fc635a5b263 in v8::Script::Run(v8::Local<v8::Context>, v8::Local<v8::Data>) src/api/api.cc:2140:7
    #20 0x55cbd927e335 in v8::Shell::ExecuteString(v8::Isolate*, v8::Local<v8::String>, v8::Local<v8::String>, v8::Shell::ReportExceptions, v8::Global<v8::Value>*) 

Summary

Type Confusion in ConvertToNumeric

Custom Questions

Type of crash:

tab

Additional Data

Category: Security
Chrome Channel: Not sure
Regression: N/A

View on issue tracker