OOB read in SharedObjectConveyorHandles::GetPersisted (issue 562567696)
夏超
Hi v8-dev,
I found an OOB read in SharedObjectConveyorHandles::GetPersisted() and filed issue 562567696. I have a one-line fix but cannot upload the CL yet, since I have no physical security key for ReAuth. Posting here first.
Bug: src/handles/shared-object-conveyor-handles.h, lines 43-46
Tagged<HeapObject> GetPersisted(uint32_t object_id) const { DCHECK(HasPersisted(object_id)); return *shared_objects_[object_id]; }
The bounds check exists but is only in a DCHECK, which is a no-op in release builds. The ID comes straight from the wire: ReadSharedObject() reads it with ReadVarint<uint32_t>() at value-serializer.cc:2604 and passes it to GetPersisted() at value-serializer.cc:2625. Persist() hands out IDs in [0, size()), so ID 1 on a one-element conveyor reads past the end.
Reproduced with a small C++ embedder (--shared-heap + AdoptSharedValueConveyor) feeding a hand-built sequence. ID 0 is fine, ID 1 aborts:
vector.h:419: libc++ Hardening assertion __n < size() failed
#7 SharedObjectConveyorHandles::GetPersisted (object_id=1) at ../../src/handles/shared-object-conveyor-handles.h:45 #8 ValueDeserializer::ReadSharedObject at ../../src/objects/value-serializer.cc:2625
object_id is 1 and the vector size is 1. Without libc++ hardening this would be a silent OOB read of a Handle<HeapObject> used as a heap object pointer.
Fix: DCHECK -> SBXCHECK (+ include "src/sandbox/check.h"). That is the macro check.h documents for an in-sandbox object holding an index into an out-of-sandbox array, and it maps to CHECK in release. Applies cleanly to current main.
Is SBXCHECK the right call, or would you rather validate the ID in ReadSharedObject() and throw a deserialization exception? Happy to send the CL either way once ReAuth is sorted.
Details and screenshots: https://issues.chromium.org/issues/562567696
Thanks, Xia Chao
Jakob Kummerow
--