Skip to content

Commit

Permalink
Should crash when deserializing JSArray object containing named prope…
Browse files Browse the repository at this point in the history 
…rty length

https://bugs.webkit.org/show_bug.cgi?id=267036
rdar://120410983

Reviewed by Sihui Liu and Mark Lam.

`length` is treated as a special property in JSArray. There shouldn't
be any named property `length` in JSArray. So, should crash when
deserializing JSArray object containing named property `length`.

* Source/WebCore/bindings/js/SerializedScriptValue.cpp:
(WebCore::CloneSerializer::serialize):
(WebCore::CloneDeserializer::objectStartVisitMember):
(WebCore::CloneDeserializer::objectEndVisitMember):
(WebCore::CloneDeserializer::deserialize):

Originally-landed-as: 272448.74@safari-7618-branch (7bd0723). rdar://124556898
Canonical link: https://commits.webkit.org/276108@main
  • Loading branch information
@hyjorc1 @robert-jenner
hyjorc1 authored and robert-jenner committed Mar 14, 2024
1 parent 9360324 commit 18b2179
Showing 1 changed file with 105 additions and 53 deletions.
158 changes: 105 additions & 53 deletions Source/WebCore/bindings/js/SerializedScriptValue.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,9 @@ enum class SerializationReturnCode {
UnspecifiedError
};

enum WalkerState { StateUnknown, ArrayStartState, ArrayStartVisitMember, ArrayEndVisitMember,
ObjectStartState, ObjectStartVisitMember, ObjectEndVisitMember,
enum WalkerState { StateUnknown, ArrayStartState, ArrayStartVisitIndexedMember, ArrayEndVisitIndexedMember,
ArrayStartVisitNamedMember, ArrayEndVisitNamedMember,
ObjectStartState, ObjectStartVisitNamedMember, ObjectEndVisitNamedMember,
MapDataStartVisitEntry, MapDataEndVisitKey, MapDataEndVisitValue,
SetDataStartVisitEntry, SetDataEndVisitKey };

Expand Down Expand Up @@ -2721,9 +2722,9 @@ SerializationReturnCode CloneSerializer::serialize(JSValue in)
indexStack.append(0);
lengthStack.append(length);
}
arrayStartVisitMember:
arrayStartVisitIndexedMember:
FALLTHROUGH;
case ArrayStartVisitMember: {
case ArrayStartVisitIndexedMember: {
JSObject* array = inputObjectStack.last();
uint32_t index = indexStack.last();
if (index == lengthStack.last()) {
Expand All @@ -2738,7 +2739,7 @@ SerializationReturnCode CloneSerializer::serialize(JSValue in)
if (propertyStack.last().size()) {
write(NonIndexPropertiesTag);
indexStack.append(0);
goto objectStartVisitMember;
goto startVisitNamedMember;
}
propertyStack.removeLast();

Expand All @@ -2751,7 +2752,7 @@ SerializationReturnCode CloneSerializer::serialize(JSValue in)
return SerializationReturnCode::ExistingExceptionError;
if (!inValue) {
indexStack.last()++;
goto arrayStartVisitMember;
goto arrayStartVisitIndexedMember;
}

write(index);
Expand All @@ -2760,15 +2761,18 @@ SerializationReturnCode CloneSerializer::serialize(JSValue in)
if (terminalCode != SerializationReturnCode::SuccessfullyCompleted)
return terminalCode;
indexStack.last()++;
goto arrayStartVisitMember;
goto arrayStartVisitIndexedMember;
}
stateStack.append(ArrayEndVisitMember);
stateStack.append(ArrayEndVisitIndexedMember);
goto stateUnknown;
}
case ArrayEndVisitMember: {
case ArrayEndVisitIndexedMember: {
indexStack.last()++;
goto arrayStartVisitMember;
goto arrayStartVisitIndexedMember;
}
case ArrayStartVisitNamedMember:
case ArrayEndVisitNamedMember:
RELEASE_ASSERT_NOT_REACHED();
objectStartState:
case ObjectStartState: {
ASSERT(inValue.isObject());
Expand All @@ -2791,9 +2795,9 @@ SerializationReturnCode CloneSerializer::serialize(JSValue in)
if (UNLIKELY(scope.exception()))
return SerializationReturnCode::ExistingExceptionError;
}
objectStartVisitMember:
startVisitNamedMember:
FALLTHROUGH;
case ObjectStartVisitMember: {
case ObjectStartVisitNamedMember: {
JSObject* object = inputObjectStack.last();
uint32_t index = indexStack.last();
PropertyNameArray& properties = propertyStack.last();
Expand All @@ -2811,7 +2815,7 @@ SerializationReturnCode CloneSerializer::serialize(JSValue in)
if (!inValue) {
// Property was removed during serialisation
indexStack.last()++;
goto objectStartVisitMember;
goto startVisitNamedMember;
}
write(properties[index]);

Expand All @@ -2820,19 +2824,19 @@ SerializationReturnCode CloneSerializer::serialize(JSValue in)

auto terminalCode = SerializationReturnCode::SuccessfullyCompleted;
if (!dumpIfTerminal(inValue, terminalCode)) {
stateStack.append(ObjectEndVisitMember);
stateStack.append(ObjectEndVisitNamedMember);
goto stateUnknown;
}
if (terminalCode != SerializationReturnCode::SuccessfullyCompleted)
return terminalCode;
FALLTHROUGH;
}
case ObjectEndVisitMember: {
case ObjectEndVisitNamedMember: {
if (UNLIKELY(scope.exception()))
return SerializationReturnCode::ExistingExceptionError;

indexStack.last()++;
goto objectStartVisitMember;
goto startVisitNamedMember;
}
mapStartState: {
ASSERT(inValue.isObject());
Expand Down Expand Up @@ -2862,7 +2866,7 @@ SerializationReturnCode CloneSerializer::serialize(JSValue in)
return SerializationReturnCode::ExistingExceptionError;
write(NonMapPropertiesTag);
indexStack.append(0);
goto objectStartVisitMember;
goto startVisitNamedMember;
}
inValue = key;
m_keepAliveBuffer.appendWithCrashOnOverflow(value);
Expand Down Expand Up @@ -2908,7 +2912,7 @@ SerializationReturnCode CloneSerializer::serialize(JSValue in)
return SerializationReturnCode::ExistingExceptionError;
write(NonSetPropertiesTag);
indexStack.append(0);
goto objectStartVisitMember;
goto startVisitNamedMember;
}
inValue = key;
stateStack.append(SetDataEndVisitKey);
Expand Down Expand Up @@ -3259,6 +3263,47 @@ class CloneDeserializer : public CloneBase {
m_version = 0xFFFFFFFF;
}

enum class VisitNamedMemberResult : uint8_t { Error, Break, Start, Unknown };

template<WalkerState endState>
ALWAYS_INLINE VisitNamedMemberResult startVisitNamedMember(MarkedVector<JSObject*, 32>& outputObjectStack, Vector<Identifier, 16>& propertyNameStack, Vector<WalkerState, 16>& stateStack, JSValue& outValue)
{
static_assert(endState == ArrayEndVisitNamedMember || endState == ObjectEndVisitNamedMember);
VM& vm = m_lexicalGlobalObject->vm();
CachedStringRef cachedString;
bool wasTerminator = false;
if (!readStringData(cachedString, wasTerminator, ShouldAtomize::Yes)) {
if (!wasTerminator) {
SERIALIZE_TRACE("FAIL deserialize");
return VisitNamedMemberResult::Error;
}

JSObject* outObject = outputObjectStack.last();
outValue = outObject;
outputObjectStack.removeLast();
return VisitNamedMemberResult::Break;
}

Identifier identifier = Identifier::fromString(vm, cachedString->string());
if constexpr (endState == ArrayEndVisitNamedMember)
RELEASE_ASSERT(identifier != vm.propertyNames->length);

if (JSValue terminal = readTerminal()) {
putProperty(outputObjectStack.last(), identifier, terminal);
return VisitNamedMemberResult::Start;
}

stateStack.append(endState);
propertyNameStack.append(identifier);
return VisitNamedMemberResult::Unknown;
}

ALWAYS_INLINE void objectEndVisitNamedMember(MarkedVector<JSObject*, 32>& outputObjectStack, Vector<Identifier, 16>& propertyNameStack, JSValue& outValue)
{
putProperty(outputObjectStack.last(), propertyNameStack.last(), outValue);
propertyNameStack.removeLast();
}

DeserializationResult deserialize();

Vector<std::optional<DetachedImageBitmap>> takeDetachedImageBitmaps() { return std::exchange(m_detachedImageBitmaps, { }); }
Expand Down Expand Up @@ -5346,9 +5391,9 @@ DeserializationResult CloneDeserializer::deserialize()
addToObjectPool<ArrayTag>(outArray);
outputObjectStack.append(outArray);
}
arrayStartVisitMember:
arrayStartVisitIndexedMember:
FALLTHROUGH;
case ArrayStartVisitMember: {
case ArrayStartVisitIndexedMember: {
uint32_t index;
if (!read(index)) {
SERIALIZE_TRACE("FAIL deserialize");
Expand All @@ -5373,7 +5418,7 @@ DeserializationResult CloneDeserializer::deserialize()
break;
}
if (index == NonIndexPropertiesTag)
goto objectStartVisitMember;
goto arrayStartVisitNamedMember;
}
} else {
if (index == TerminatorTag) {
Expand All @@ -5382,24 +5427,42 @@ DeserializationResult CloneDeserializer::deserialize()
outputObjectStack.removeLast();
break;
} else if (index == NonIndexPropertiesTag)
goto objectStartVisitMember;
goto arrayStartVisitNamedMember;
}

if (JSValue terminal = readTerminal()) {
putProperty(outputObjectStack.last(), index, terminal);
goto arrayStartVisitMember;
goto arrayStartVisitIndexedMember;
}
if (m_failed)
goto error;
indexStack.append(index);
stateStack.append(ArrayEndVisitMember);
stateStack.append(ArrayEndVisitIndexedMember);
goto stateUnknown;
}
case ArrayEndVisitMember: {
case ArrayEndVisitIndexedMember: {
JSObject* outArray = outputObjectStack.last();
putProperty(outArray, indexStack.last(), outValue);
indexStack.removeLast();
goto arrayStartVisitMember;
goto arrayStartVisitIndexedMember;
}
arrayStartVisitNamedMember:
case ArrayStartVisitNamedMember: {
switch (startVisitNamedMember<ArrayEndVisitNamedMember>(outputObjectStack, propertyNameStack, stateStack, outValue)) {
case VisitNamedMemberResult::Error:
goto error;
case VisitNamedMemberResult::Break:
break;
case VisitNamedMemberResult::Start:
goto arrayStartVisitNamedMember;
case VisitNamedMemberResult::Unknown:
goto stateUnknown;
}
break;
}
case ArrayEndVisitNamedMember: {
objectEndVisitNamedMember(outputObjectStack, propertyNameStack, outValue);
goto arrayStartVisitNamedMember;
}
objectStartState:
case ObjectStartState: {
Expand All @@ -5409,35 +5472,24 @@ DeserializationResult CloneDeserializer::deserialize()
addToObjectPool<ObjectTag>(outObject);
outputObjectStack.append(outObject);
}
objectStartVisitMember:
startVisitNamedMember:
FALLTHROUGH;
case ObjectStartVisitMember: {
CachedStringRef cachedString;
bool wasTerminator = false;
if (!readStringData(cachedString, wasTerminator, ShouldAtomize::Yes)) {
if (!wasTerminator) {
SERIALIZE_TRACE("FAIL deserialize");
goto error;
}

JSObject* outObject = outputObjectStack.last();
outValue = outObject;
outputObjectStack.removeLast();
case ObjectStartVisitNamedMember: {
switch (startVisitNamedMember<ObjectEndVisitNamedMember>(outputObjectStack, propertyNameStack, stateStack, outValue)) {
case VisitNamedMemberResult::Error:
goto error;
case VisitNamedMemberResult::Break:
break;
case VisitNamedMemberResult::Start:
goto startVisitNamedMember;
case VisitNamedMemberResult::Unknown:
goto stateUnknown;
}

if (JSValue terminal = readTerminal()) {
putProperty(outputObjectStack.last(), Identifier::fromString(vm, cachedString->string()), terminal);
goto objectStartVisitMember;
}
stateStack.append(ObjectEndVisitMember);
propertyNameStack.append(Identifier::fromString(vm, cachedString->string()));
goto stateUnknown;
break;
}
case ObjectEndVisitMember: {
putProperty(outputObjectStack.last(), propertyNameStack.last(), outValue);
propertyNameStack.removeLast();
goto objectStartVisitMember;
case ObjectEndVisitNamedMember: {
objectEndVisitNamedMember(outputObjectStack, propertyNameStack, outValue);
goto startVisitNamedMember;
}
mapStartState: {
if (outputObjectStack.size() > maximumFilterRecursion) {
Expand All @@ -5454,7 +5506,7 @@ DeserializationResult CloneDeserializer::deserialize()
case MapDataStartVisitEntry: {
if (consumeCollectionDataTerminationIfPossible<NonMapPropertiesTag>()) {
mapStack.removeLast();
goto objectStartVisitMember;
goto startVisitNamedMember;
}
stateStack.append(MapDataEndVisitKey);
goto stateUnknown;
Expand Down Expand Up @@ -5485,7 +5537,7 @@ DeserializationResult CloneDeserializer::deserialize()
case SetDataStartVisitEntry: {
if (consumeCollectionDataTerminationIfPossible<NonSetPropertiesTag>()) {
setStack.removeLast();
goto objectStartVisitMember;
goto startVisitNamedMember;
}
stateStack.append(SetDataEndVisitKey);
goto stateUnknown;
Expand Down

0 comments on commit 18b2179

Please sign in to comment.