Skip to content

Commit

Permalink
CloneDeserializer::readBigInt() should check for overflow when reifyi…
Browse files Browse the repository at this point in the history
…ng JSBigInt length on 32-bit platforms.

https://bugs.webkit.org/show_bug.cgi?id=260822
rdar://114547822

Reviewed by Chris Dumez.

The serialized length is a number of Uint64 elements. On 32-bit platforms, this length gets multiplied by 2 in
order to compute the actual length of the backing store to re-construct the JSBigInt.  Both the transmitted length
and the JSBigInt length is stored as uint32_t.  Hence, the 2x multiplication can theoretically result in an
overflow.  This patch adds an overflow check to handle this edge case.

Also renamed lengthInUint64 to numberOfUint64Elements.  lengthInUint64 can be misread to mean a length stored as a
uint64_t value, which is not what it actually means.  The length value is store in a uint32_t, and is a count of
the number of uint64_t sized elements.

* Source/WebCore/bindings/js/SerializedScriptValue.cpp:
(WebCore::CloneSerializer::dumpHeapBigIntData):
(WebCore::CloneDeserializer::readBigInt):

Originally-landed-as: 265870.467@safari-7616-branch (1f9e212). rdar://117809900
Canonical link: https://commits.webkit.org/270127@main
  • Loading branch information
Mark Lam authored and JonWBedard committed Nov 2, 2023
1 parent c180d85 commit c89614c
Showing 1 changed file with 17 additions and 12 deletions.
29 changes: 17 additions & 12 deletions Source/WebCore/bindings/js/SerializedScriptValue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,7 @@ static constexpr unsigned StringDataIs8BitFlag = 0x80000000;
* BigIntObjectTag BigIntData
*
* BigIntData :-
* <sign:uint8_t> <lengthInUint64:uint32_t> <contents:uint64_t{lengthInUint64}>
* <sign:uint8_t> <numberOfUint64Elements:uint32_t> <contents:uint64_t{numberOfUint64Elements}>
*
* File :-
* FileTag FileData
Expand Down Expand Up @@ -1152,10 +1152,10 @@ class CloneSerializer : CloneBase {
write(static_cast<uint64_t>(bigInt->digit(index)));
} else {
ASSERT(sizeof(JSBigInt::Digit) == sizeof(uint32_t));
uint32_t lengthInUint64 = bigInt->length() / 2;
uint32_t numberOfUint64Elements = bigInt->length() / 2;
if (bigInt->length() & 0x1)
++lengthInUint64;
write(lengthInUint64);
++numberOfUint64Elements;
write(numberOfUint64Elements);
uint64_t value = 0;
for (unsigned index = 0; index < bigInt->length(); ++index) {
if (!(index & 0x1))
Expand Down Expand Up @@ -4173,11 +4173,11 @@ class CloneDeserializer : CloneBase {
bool sign;
if (!read(sign, ForceReadingAs8Bit::Yes))
return JSValue();
uint32_t lengthInUint64 = 0;
if (!read(lengthInUint64))
uint32_t numberOfUint64Elements = 0;
if (!read(numberOfUint64Elements))
return JSValue();

if (!lengthInUint64) {
if (!numberOfUint64Elements) {
#if USE(BIGINT32)
return jsBigInt32(0);
#else
Expand All @@ -4193,7 +4193,7 @@ class CloneDeserializer : CloneBase {

#if USE(BIGINT32)
static_assert(sizeof(JSBigInt::Digit) == sizeof(uint64_t));
if (lengthInUint64 == 1) {
if (numberOfUint64Elements == 1) {
uint64_t digit64 = 0;
if (!read(digit64))
return JSValue();
Expand Down Expand Up @@ -4223,25 +4223,30 @@ class CloneDeserializer : CloneBase {
#endif
JSBigInt* bigInt = nullptr;
if constexpr (sizeof(JSBigInt::Digit) == sizeof(uint64_t)) {
bigInt = JSBigInt::tryCreateWithLength(m_lexicalGlobalObject->vm(), lengthInUint64);
bigInt = JSBigInt::tryCreateWithLength(m_lexicalGlobalObject->vm(), numberOfUint64Elements);
if (!bigInt) {
fail();
return JSValue();
}
for (uint32_t index = 0; index < lengthInUint64; ++index) {
for (uint32_t index = 0; index < numberOfUint64Elements; ++index) {
uint64_t digit64 = 0;
if (!read(digit64))
return JSValue();
bigInt->setDigit(index, digit64);
}
} else {
ASSERT(sizeof(JSBigInt::Digit) == sizeof(uint32_t));
bigInt = JSBigInt::tryCreateWithLength(m_lexicalGlobalObject->vm(), lengthInUint64 * 2);
auto actualBigIntLength = WTF::checkedProduct<uint32_t>(numberOfUint64Elements, 2);
if (actualBigIntLength.hasOverflowed()) {
fail();
return JSValue();
}
bigInt = JSBigInt::tryCreateWithLength(m_lexicalGlobalObject->vm(), actualBigIntLength.value());
if (!bigInt) {
fail();
return JSValue();
}
for (uint32_t index = 0; index < lengthInUint64; ++index) {
for (uint32_t index = 0; index < numberOfUint64Elements; ++index) {
uint64_t digit64 = 0;
if (!read(digit64))
return JSValue();
Expand Down

0 comments on commit c89614c

Please sign in to comment.