Skip to content

Commit dd5b2e7

Browse files
Thomas Moore (CHAKRA)MikeHolman
authored andcommitted
[CVE-2018-8367] Edge - UAF for Edge on WIP - Qihoo 360
This change fixes an issue where deepCopied arrays have a different inlined head segment behavior from its original instance. This is because deepCopy'ing an array would unconditionally allocate the head segment as inline, regardless of its size. This conflicts with JavascriptArray::HasInlineHeadSegment, which is based upon size rather than layout. In the PoC, this causes problems when removing elements via Slice. The fix is to clone the inlined behavior of the original instance in addition to the instance's data. This allows related invariants to be maintained. This change also ensures that the aligned, allocated size of the inline head segment on the stack is never larger than INLINE_CHUNK_SIZE, similarly to NewLiteral.
1 parent e035a2d commit dd5b2e7

File tree

3 files changed

+57
-17
lines changed

3 files changed

+57
-17
lines changed

lib/Backend/Lower.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3880,6 +3880,16 @@ Lowerer::GenerateArrayAllocHelper(IR::Instr *instr, uint32 * psize, Js::ArrayCal
38803880
uint32 allocCount = count == 0 ? Js::SparseArraySegmentBase::SMALL_CHUNK_SIZE : count;
38813881
arrayAllocSize = Js::JavascriptArray::DetermineAllocationSize<ArrayType, 0>(allocCount, nullptr, &alignedHeadSegmentSize);
38823882
}
3883+
3884+
// Note that it is possible for the returned alignedHeadSegmentSize to be greater than INLINE_CHUNK_SIZE because
3885+
// of rounding the *entire* object, including the head segment, to the nearest aligned size. In that case, ensure
3886+
// that this size is still not larger than INLINE_CHUNK_SIZE size because the head segment is still inlined. This
3887+
// keeps consistency with the definition of HasInlineHeadSegment and maintained in the assert below.
3888+
uint inlineChunkSize = Js::SparseArraySegmentBase::INLINE_CHUNK_SIZE;
3889+
alignedHeadSegmentSize = min(alignedHeadSegmentSize, inlineChunkSize);
3890+
3891+
Assert(ArrayType::HasInlineHeadSegment(alignedHeadSegmentSize));
3892+
38833893
leaHeadInstr = IR::Instr::New(Js::OpCode::LEA, headOpnd,
38843894
IR::IndirOpnd::New(dstOpnd, sizeof(ArrayType), TyMachPtr, func), func);
38853895
isHeadSegmentZeroed = true;

lib/Runtime/Library/JavascriptArray.cpp

Lines changed: 46 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -11814,24 +11814,42 @@ using namespace Js;
1181411814
#endif
1181511815

1181611816
template <typename T>
11817-
void JavascriptArray::InitBoxedInlineSegments(SparseArraySegment<T> * dst, SparseArraySegment<T> * src, bool deepCopy)
11817+
void JavascriptArray::InitBoxedInlineSegments(T * instance, bool deepCopy)
1181811818
{
1181911819
// Don't copy the segment map, we will build it again
1182011820
SetFlags(GetFlags() & ~DynamicObjectFlags::HasSegmentMap);
1182111821

11822-
SetHeadAndLastUsedSegment(dst);
11822+
SparseArraySegment<typename T::TElement>* src = SparseArraySegment<typename T::TElement>::From(instance->head);
11823+
SparseArraySegment<typename T::TElement>* dst;
11824+
11825+
if (IsInlineSegment(src, instance))
11826+
{
11827+
Assert(src->size <= SparseArraySegmentBase::INLINE_CHUNK_SIZE);
11828+
11829+
// Copy head segment data between inlined head segments
11830+
dst = DetermineInlineHeadSegmentPointer<T, 0, true>(static_cast<T*>(this));
11831+
dst->left = src->left;
11832+
dst->length = src->length;
11833+
dst->size = src->size;
11834+
}
11835+
else
11836+
{
11837+
// Otherwise, ensure that the new head segment is allocated now in the recycler so that the data can be copied.
11838+
// Note: src->next is provided to control whether a leaf segment is allocated just as it is with instance. If
11839+
// src->next is non-null, the appropriate update to dst->next will continue below.
11840+
dst = SparseArraySegment<typename T::TElement>::AllocateSegment(GetRecycler(), src->left, src->length, src->size, src->next);
11841+
}
1182311842

11824-
// Copy head segment data
11825-
dst->left = src->left;
11826-
dst->length = src->length;
11827-
dst->size = src->size;
11843+
SetHeadAndLastUsedSegment(dst);
1182811844
dst->CheckLengthvsSize();
1182911845

11846+
Assert(IsInlineSegment(src, instance) == IsInlineSegment(dst, static_cast<T*>(this)));
11847+
1183011848
CopyArray(dst->elements, dst->size, src->elements, src->size);
1183111849

1183211850
if (!deepCopy)
1183311851
{
11834-
// Without a deep copy, point to the existing next segment
11852+
// Without a deep copy, point to the existing next segment from the original instance
1183511853
dst->next = src->next;
1183611854
}
1183711855
else
@@ -11845,10 +11863,10 @@ using namespace Js;
1184511863
{
1184611864
// Allocate a new segment in the destination and copy from src
1184711865
// note: PointerValue is to strip SWB wrapping before static_cast
11848-
src = static_cast<SparseArraySegment<T>*>(PointerValue(src->next));
11866+
src = static_cast<SparseArraySegment<typename T::TElement>*>(PointerValue(src->next));
1184911867

1185011868
dst->next = dst->AllocateSegment(GetRecycler(), src->left, src->length, src->size, src->next);
11851-
dst = static_cast<SparseArraySegment<T>*>(PointerValue(dst->next));
11869+
dst = static_cast<SparseArraySegment<typename T::TElement>*>(PointerValue(dst->next));
1185211870

1185311871
CopyArray(dst->elements, dst->size, src->elements, src->size);
1185411872
}
@@ -11861,14 +11879,20 @@ using namespace Js;
1186111879
} while (dst != nullptr);
1186211880
failFastError.Completed();
1186311881
}
11882+
11883+
// Assert either
11884+
// - there is only the head segment
11885+
// - the new head segment points to a new next segment
11886+
// - the new head segment points to the existing next segment because this is not a deepCopy
11887+
Assert(this->head->next == nullptr || this->head->next != src->next || !deepCopy);
1186411888
}
1186511889

1186611890
JavascriptArray::JavascriptArray(JavascriptArray * instance, bool boxHead, bool deepCopy)
1186711891
: ArrayObject(instance, deepCopy)
1186811892
{
1186911893
if (boxHead)
1187011894
{
11871-
InitBoxedInlineSegments(DetermineInlineHeadSegmentPointer<JavascriptArray, 0, true>(this), SparseArraySegment<Var>::From(instance->head), false);
11895+
InitBoxedInlineSegments(instance, deepCopy);
1187211896
}
1187311897
else
1187411898
{
@@ -11880,13 +11904,19 @@ using namespace Js;
1188011904
}
1188111905

1188211906
// Allocate a new Array with its own segments and copy the data in instance
11883-
// into the new Array
11907+
// into the new Array. If the instance being deepCopy'd has an inline head
11908+
// segment, then make sure the new instance also has allocation for an inline
11909+
// head segment.
1188411910
template <typename T>
1188511911
T * JavascriptArray::DeepCopyInstance(T * instance)
1188611912
{
11887-
return RecyclerNewPlusZ(instance->GetRecycler(),
11888-
instance->GetTypeHandler()->GetInlineSlotsSize() + sizeof(Js::SparseArraySegmentBase) + instance->head->size * sizeof(typename T::TElement),
11889-
T, instance, true /*boxHead*/, true /*deepCopy*/);
11913+
size_t allocSize = instance->GetTypeHandler()->GetInlineSlotsSize();
11914+
if (IsInlineSegment(instance->head, instance))
11915+
{
11916+
allocSize += sizeof(Js::SparseArraySegmentBase) + instance->head->size * sizeof(typename T::TElement);
11917+
}
11918+
11919+
return RecyclerNewPlusZ(instance->GetRecycler(), allocSize, T, instance, true /*boxHead*/, true /*deepCopy*/);
1189011920
}
1189111921

1189211922
ArrayObject* JavascriptArray::DeepCopyInstance(ArrayObject* arrayObject)
@@ -12051,7 +12081,7 @@ using namespace Js;
1205112081
{
1205212082
if (boxHead)
1205312083
{
12054-
InitBoxedInlineSegments(DetermineInlineHeadSegmentPointer<JavascriptNativeIntArray, 0, true>(this), SparseArraySegment<int>::From(instance->head), deepCopy);
12084+
InitBoxedInlineSegments(instance, deepCopy);
1205512085
}
1205612086
else
1205712087
{
@@ -12097,7 +12127,7 @@ using namespace Js;
1209712127
{
1209812128
if (boxHead)
1209912129
{
12100-
InitBoxedInlineSegments(DetermineInlineHeadSegmentPointer<JavascriptNativeFloatArray, 0, true>(this), SparseArraySegment<double>::From(instance->head), deepCopy);
12130+
InitBoxedInlineSegments(instance, deepCopy);
1210112131
}
1210212132
else
1210312133
{

lib/Runtime/Library/JavascriptArray.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -904,7 +904,7 @@ namespace Js
904904
static JavascriptArray * BoxStackInstance(JavascriptArray * instance, bool deepCopy);
905905
static ArrayObject * DeepCopyInstance(ArrayObject * instance);
906906
protected:
907-
template <typename T> void InitBoxedInlineSegments(SparseArraySegment<T> * dst, SparseArraySegment<T> * src, bool deepCopy);
907+
template <typename T> void InitBoxedInlineSegments(T * instance, bool deepCopy);
908908

909909
template <typename T> static T * BoxStackInstance(T * instance, bool deepCopy);
910910
template <typename T> static T * DeepCopyInstance(T * instance);

0 commit comments

Comments
 (0)