[sandbox] Fix EmbedderDataSlot::ToAlignedPointerSafe

We need to properly handle the case of uninitialized embedder data slots
which contain the "undefined" value and thus might look like valid
external pointer table indices.

Bug: v8:10391
Change-Id: I169a3e42132dde223ea151c1a5d5956c72341f8d
Cq-Include-Trybots: luci.v8.try:v8_linux64_heap_sandbox_dbg_ng,v8_linux_arm64_sim_heap_sandbox_dbg_ng
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3448378Reviewed-by: Igor Sheludko <ishell@chromium.org>
Commit-Queue: Samuel Groß <saelo@chromium.org>
Cr-Commit-Position: refs/heads/main@{#79009}

src/objects/embedder-data-slot-inl.h

@@ -118,7 +118,14 @@ bool EmbedderDataSlot::ToAlignedPointerSafe(Isolate* isolate,
     raw_value = isolate->external_pointer_table().Get(
         index, kEmbedderDataSlotPayloadTag);
     *out_pointer = reinterpret_cast<void*>(raw_value);
-    return true;
+    // The index being valid does not guarantee that this slot contains an
+    // external pointer. After initialization, the raw part will contain the
+    // "undefined" value (see Factory::InitializeJSObjectBody) which could look
+    // like an external pointer table index as well. To deal with that, we also
+    // check that the returned value has the embedder data slot tag, since
+    // otherwise the pointer would be invalid.
+    // TODO(v8:10391) maybe initialize the slot to zero to avoid this issue.
+    return (raw_value & kExternalPointerTagMask) == 0;
   }
   return false;
 #else

src/objects/embedder-data-slot.h

@@ -85,12 +85,13 @@ class EmbedderDataSlot
   V8_INLINE bool ToAlignedPointer(Isolate* isolate, void** out_result) const;
 
   // Same as ToAlignedPointer() but with a workaround for sandboxed external
-  // pointers.  When sandboxed external pointers are enabled, this method
+  // pointers. When sandboxed external pointers are enabled, this method
   // doesn't crash when the raw part of the slot contains "undefined" instead
-  // of a correct external table entry index (see
-  // Factory::InitializeJSObjectBody() for details).  Returns true when the
-  // external pointer table index was pointing to a valid entry, otherwise
-  // false.
+  // of a valid external table entry index (see
+  // Factory::InitializeJSObjectBody() for details). Returns true if this slot
+  // contains a valid external pointer, false otherwise.
+  //
+  // TODO(v8:10391) we could instead initialize the raw part to zero.
   //
   // Call this function if you are not sure whether the slot contains valid
   // external pointer or not.