[regexp] Secure interpreter dispatch.

Currently the dispatch table could be accessed out of bounds if something
is wrong with the generated bytecode.
OOB access of the dispatch table can lead to jumps to arbitrary addresses
in the code space.

This CL prevents this issue by changing the following:
BYTECODE_MASK now filters out all bits not currently used for bytecodes.
All unused slots between the last actually defined bytecode and
BYTECODE_MASK are now filled with BREAK Bytecodes (invalid operation).
This way we can not access out of bounds of the dispatch table if
something is broken/tampered with, preventing jumps to arbitrary code.

Bug: v8:9699
Change-Id: Ibce591ae94b52472ba74a9fd0666e55185af7b2c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1795349
Commit-Queue: Patrick Thier <pthier@google.com>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#63708}

src/regexp/regexp-bytecodes.h

@@ -10,12 +10,19 @@
 namespace v8 {
 namespace internal {
 
-const int BYTECODE_MASK = 0xff;
+// Maximum number of bytecodes that will be used (next power of 2 of actually
+// defined bytecodes).
+// All slots between the last actually defined bytecode and maximum id will be
+// filled with BREAKs, indicating an invalid operation. This way using
+// BYTECODE_MASK guarantees no OOB access to the dispatch table.
+constexpr int kRegExpPaddedBytecodeCount = 1 << 6;
+constexpr int BYTECODE_MASK = kRegExpPaddedBytecodeCount - 1;
 // The first argument is packed in with the byte code in one word, but so it
 // has 24 bits, but it can be positive and negative so only use 23 bits for
 // positive values.
 const unsigned int MAX_FIRST_ARG = 0x7fffffu;
 const int BYTECODE_SHIFT = 8;
+STATIC_ASSERT(1 << BYTECODE_SHIFT > BYTECODE_MASK);
 
 #define BYTECODE_ITERATOR(V)                                                   \
   V(BREAK, 0, 4)              /* bc8                                        */ \

src/regexp/regexp-interpreter.cc

@@ -297,11 +297,58 @@ IrregexpInterpreter::Result RawMatch(Isolate* isolate, ByteArray code_array,
   DisallowHeapAllocation no_gc;
 
 #if V8_USE_COMPUTED_GOTO
-#define DECLARE_DISPATCH_TABLE_ENTRY(name, code, length) &&BC_##name,
-  static const void* const dispatch_table[] = {
-      BYTECODE_ITERATOR(DECLARE_DISPATCH_TABLE_ENTRY)};
+
+// We have to make sure that no OOB access to the dispatch table is possible and
+// all values are valid label addresses.
+// Otherwise jumps to arbitrary addresses could potentially happen.
+// This is ensured as follows:
+// Every index to the dispatch table gets masked using BYTECODE_MASK in
+// DECODE(). This way we can only get values between 0 (only the least
+// significant byte of an integer is used) and kRegExpPaddedBytecodeCount - 1
+// (BYTECODE_MASK is defined to be exactly this value).
+// All entries from kRegExpBytecodeCount to kRegExpPaddedBytecodeCount have to
+// be filled with BREAKs (invalid operation).
+
+// Fill dispatch table from last defined bytecode up to the next power of two
+// with BREAK (invalid operation).
+// TODO(pthier): Find a way to fill up automatically (at compile time)
+// 53 real bytecodes -> 11 fillers
+#define BYTECODE_FILLER_ITERATOR(V) \
+  V(BREAK) /* 1 */                  \
+  V(BREAK) /* 2 */                  \
+  V(BREAK) /* 3 */                  \
+  V(BREAK) /* 4 */                  \
+  V(BREAK) /* 5 */                  \
+  V(BREAK) /* 6 */                  \
+  V(BREAK) /* 7 */                  \
+  V(BREAK) /* 8 */                  \
+  V(BREAK) /* 9 */                  \
+  V(BREAK) /* 10 */                 \
+  V(BREAK) /* 11 */
+
+#define COUNT(...) +1
+  static constexpr int kRegExpBytecodeFillerCount =
+      BYTECODE_FILLER_ITERATOR(COUNT);
+#undef COUNT
+
+  // Make sure kRegExpPaddedBytecodeCount is actually the closest possible power
+  // of two.
+  DCHECK_EQ(kRegExpPaddedBytecodeCount,
+            base::bits::RoundUpToPowerOfTwo32(kRegExpBytecodeCount));
+
+  // Make sure every bytecode we get by using BYTECODE_MASK is well defined.
+  STATIC_ASSERT(kRegExpBytecodeCount <= kRegExpPaddedBytecodeCount);
+  STATIC_ASSERT(kRegExpBytecodeCount + kRegExpBytecodeFillerCount ==
+                kRegExpPaddedBytecodeCount);
+
+#define DECLARE_DISPATCH_TABLE_ENTRY(name, ...) &&BC_##name,
+  static const void* const dispatch_table[kRegExpPaddedBytecodeCount] = {
+      BYTECODE_ITERATOR(DECLARE_DISPATCH_TABLE_ENTRY)
+          BYTECODE_FILLER_ITERATOR(DECLARE_DISPATCH_TABLE_ENTRY)};
 #undef DECLARE_DISPATCH_TABLE_ENTRY
-#endif
+#undef BYTECODE_FILLER_ITERATOR
+
+#endif  // V8_USE_COMPUTED_GOTO
 
   const byte* pc = code_array.GetDataStartAddress();
   const byte* code_base = pc;