Skip to content

Commit

Permalink
Change to address CVE-2016-7200,CVE-2016-7201,CVE-2016-7202,CVE-2016-…
Browse files Browse the repository at this point in the history 
…7203,CVE-2016-7208,CVE-2016-7240,CVE-2016-7241,CVE-2016-7242,CVE-2016-7243

Type confusion in Array.prototype.filter
Type confusion due to reentrancy can cause a Var to be written into a native int array.
Fix by making sure type-specialized code path is used only when ArraySpeciesCreate() invokes
built-in Array constructor.

Heap overflow in Array.prototype.splice
In Array.prototype.splice, array length is cached before ArraySpeciesCreate() is invoked.
Side-effect from ArraySpeciesCreate() can change array length and result in inconsistent states
and possibly heap overflow.
Fix by adding length check to keep cases with side effects out of fast path with pre-calculated length.
Also tweak logic in ArraySpeciesCreate() to flag a non-built-in constructor with missing [@species] property.

Type confusion in FillFromPrototypes
In ForEachOwnMissingArrayIndexOfObject(), existing array enumeration logic assumes Var array.
A native array from caller can cause type confusion and leak.
Fix by converting incoming native arrays to Var arrays.

Parameter type confusion in eval
Extra argument signified by CallFlags_ExtraArg shall be cast to FrameDisplay unless the extra argument
is used for new.target, in which case CallFlags_NewTarget is be set. Type confusion and AV occur because
existing logic in eval() does not check if CallFlags_NewTarget is cleared before using extra argument as
FrameDisplay.
Fix by adding CallFlags_NewTarget check to eval() before cast to FrameDisplay.

Type confusion in JSON.parse
Non-native array is expected in JSONParser::Walk(). A native array from caller
can cause type confusion and heap overflow
Fix by converting native arrays to Var arrays.

Type confusion in Array.prototype.concat and .splice
Array newly created by ArraySpeciesCreate is not being checked if it is a
JavascriptCopyOnAccessNativeIntArray, causing near-nullptr AVs.
Fix by adding check-and-convert against JavascriptCopyOnAccessNativeIntArray in
affected built-ins.
  • Loading branch information
Suwei Chen committed Nov 11, 2016
1 parent 103f02f commit c2787ef
Show file tree
Hide file tree
Showing 7 changed files with 274 additions and 27 deletions.
10 changes: 10 additions & 0 deletions lib/Runtime/Base/CallInfo.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,16 @@ namespace Js
static const ushort ksizeofCount;
static const ushort ksizeofCallFlags;
static const uint kMaxCountArgs;

static bool isDirectEvalCall(CallFlags flags)
{
// This was recognized as an eval call at compile time. The last one or two args are internal to us.
// Argcount will be one of the following when called from global code
// - eval("...") : argcount 3 : this, evalString, frameDisplay
// - eval.call("..."): argcount 2 : this(which is string) , frameDisplay

return (flags & (CallFlags_ExtraArg | CallFlags_NewTarget)) == CallFlags_ExtraArg; // ExtraArg == 1 && NewTarget == 0
}
};

struct InlineeCallInfo
Expand Down
2 changes: 1 addition & 1 deletion lib/Runtime/Library/GlobalObject.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -491,7 +491,7 @@ namespace Js
// TODO: Handle call from global scope, strict mode
BOOL isIndirect = FALSE;

if (args.Info.Flags & CallFlags_ExtraArg)
if (Js::CallInfo::isDirectEvalCall(args.Info.Flags))
{
// This was recognized as an eval call at compile time. The last one or two args are internal to us.
// Argcount will be one of the following when called from global code
Expand Down
3 changes: 1 addition & 2 deletions lib/Runtime/Library/JSONParser.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,7 @@ namespace JSON
// this is a post order walk. Visit the children before calling walk on this object
if (Js::DynamicObject::IsAnyArray(value))
{
Js::JavascriptArray* arrayVal = Js::JavascriptArray::FromAnyArray(value);
// REVIEW: How do we guarantee that JSON objects are not native arrays?
Js::JavascriptArray* arrayVal = JavascriptArray::EnsureNonNativeArray(Js::JavascriptArray::FromAnyArray(value));
Assert(!Js::JavascriptNativeIntArray::Is(arrayVal) && !Js::JavascriptNativeFloatArray::Is(arrayVal));
uint length = arrayVal->GetLength();
if (!arrayVal->IsCrossSiteObject())
Expand Down
92 changes: 72 additions & 20 deletions lib/Runtime/Library/JavascriptArray.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -714,6 +714,40 @@ namespace Js
return IsMissingHeadSegmentItemImpl<double>(index);
}

template<typename T>
void JavascriptArray::InternalFillFromPrototype(JavascriptArray *dstArray, const T& dstIndex, JavascriptArray *srcArray, uint32 start, uint32 end, uint32 count)
{
RecyclableObject* prototype = srcArray->GetPrototype();
while (start + count != end && JavascriptOperators::GetTypeId(prototype) != TypeIds_Null)
{
ForEachOwnMissingArrayIndexOfObject(srcArray, dstArray, prototype, start, end, dstIndex, [&](uint32 index, Var value) {
T n = dstIndex + (index - start);
dstArray->DirectSetItemAt(n, value);

count++;
});

prototype = prototype->GetPrototype();
}
}

template<>
void JavascriptArray::InternalFillFromPrototype<uint32>(JavascriptArray *dstArray, const uint32& dstIndex, JavascriptArray *srcArray, uint32 start, uint32 end, uint32 count)
{
RecyclableObject* prototype = srcArray->GetPrototype();
while (start + count != end && JavascriptOperators::GetTypeId(prototype) != TypeIds_Null)
{
ForEachOwnMissingArrayIndexOfObject(srcArray, dstArray, prototype, start, end, dstIndex, [&](uint32 index, Var value) {
uint32 n = dstIndex + (index - start);
dstArray->SetItem(n, value, PropertyOperation_None);

count++;
});

prototype = prototype->GetPrototype();
}
}

/* static */
bool JavascriptArray::HasInlineHeadSegment(uint32 length)
{
Expand Down Expand Up @@ -3308,6 +3342,9 @@ namespace Js
pDestObj = ArraySpeciesCreate(args[0], 0, scriptContext);
if (pDestObj)
{
#if ENABLE_COPYONACCESS_ARRAY
JavascriptLibrary::CheckAndConvertCopyOnAccessNativeIntArray<Var>(pDestObj);
#endif
// Check the thing that species create made. If it's a native array that can't handle the source
// data, convert it. If it's a more conservative kind of array than the source data, indicate that
// so that the data will be converted on copy.
Expand Down Expand Up @@ -5863,13 +5900,19 @@ namespace Js
}

newArr = CreateNewArrayHelper(static_cast<uint32>(newLenT), isIntArray, isFloatArray, pArr, scriptContext);
#if ENABLE_COPYONACCESS_ARRAY
JavascriptLibrary::CheckAndConvertCopyOnAccessNativeIntArray<Var>(newArr);
#endif
newObj = newArr;
}
else
{
// If the new object we created is an array, remember that as it will save us time setting properties in the object below
if (JavascriptArray::Is(newObj))
{
#if ENABLE_COPYONACCESS_ARRAY
JavascriptLibrary::CheckAndConvertCopyOnAccessNativeIntArray<Var>(newObj);
#endif
newArr = JavascriptArray::FromVar(newObj);
}
}
Expand Down Expand Up @@ -6649,6 +6692,9 @@ namespace Js
// If the new object we created is an array, remember that as it will save us time setting properties in the object below
if (JavascriptArray::Is(newObj))
{
#if ENABLE_COPYONACCESS_ARRAY
JavascriptLibrary::CheckAndConvertCopyOnAccessNativeIntArray<Var>(newObj);
#endif
newArr = JavascriptArray::FromVar(newObj);
}
}
Expand All @@ -6657,10 +6703,13 @@ namespace Js
{
pArr->GetArrayTypeAndConvert(&isIntArray, &isFloatArray);
newArr = CreateNewArrayHelper(deleteLen, isIntArray, isFloatArray, pArr, scriptContext);
#if ENABLE_COPYONACCESS_ARRAY
JavascriptLibrary::CheckAndConvertCopyOnAccessNativeIntArray<Var>(newArr);
#endif
}

// If return object is a JavascriptArray, we can use all the array splice helpers
if (newArr && isBuiltinArrayCtor)
if (newArr && isBuiltinArrayCtor && len == pArr->length)
{

// Array has a single segment (need not start at 0) and splice start lies in the range
Expand Down Expand Up @@ -7151,6 +7200,9 @@ namespace Js

if (JavascriptArray::Is(pNewObj))
{
#if ENABLE_COPYONACCESS_ARRAY
JavascriptLibrary::CheckAndConvertCopyOnAccessNativeIntArray<Var>(pNewObj);
#endif
pnewArr = JavascriptArray::FromVar(pNewObj);
}

Expand Down Expand Up @@ -8924,6 +8976,9 @@ namespace Js
// If the new object we created is an array, remember that as it will save us time setting properties in the object below
if (JavascriptArray::Is(newObj))
{
#if ENABLE_COPYONACCESS_ARRAY
JavascriptLibrary::CheckAndConvertCopyOnAccessNativeIntArray<Var>(newObj);
#endif
newArr = JavascriptArray::FromVar(newObj);
}
}
Expand Down Expand Up @@ -9123,7 +9178,8 @@ namespace Js
}

// If the source object is an Array exotic object we should try to load the constructor property and use it to construct the return object.
RecyclableObject* newObj = ArraySpeciesCreate(obj, 0, scriptContext);
bool isBuiltinArrayCtor = true;
RecyclableObject* newObj = ArraySpeciesCreate(obj, 0, scriptContext, nullptr, nullptr, &isBuiltinArrayCtor);
JavascriptArray* newArr = nullptr;

if (newObj == nullptr)
Expand All @@ -9137,6 +9193,9 @@ namespace Js
// If the new object we created is an array, remember that as it will save us time setting properties in the object below
if (JavascriptArray::Is(newObj))
{
#if ENABLE_COPYONACCESS_ARRAY
JavascriptLibrary::CheckAndConvertCopyOnAccessNativeIntArray<Var>(newObj);
#endif
newArr = JavascriptArray::FromVar(newObj);
}
}
Expand Down Expand Up @@ -9164,7 +9223,7 @@ namespace Js
if (JavascriptConversion::ToBoolean(selected, scriptContext))
{
// Try to fast path if the return object is an array
if (newArr)
if (newArr && isBuiltinArrayCtor)
{
newArr->DirectSetItemAt(i, element);
}
Expand Down Expand Up @@ -10041,6 +10100,7 @@ namespace Js
{
if (JavascriptArray::Is(arr))
{
arr = EnsureNonNativeArray(arr);
ArrayElementEnumerator e(arr, startIndex, limitIndex);

while(e.MoveNext<Var>())
Expand Down Expand Up @@ -10096,6 +10156,7 @@ namespace Js
{
if (JavascriptArray::Is(arr))
{
arr = EnsureNonNativeArray(arr);
ArrayElementEnumerator e(arr, startIndex, limitIndex);

while(e.MoveNext<Var>())
Expand Down Expand Up @@ -10849,6 +10910,9 @@ namespace Js

JavascriptArray *JavascriptArray::EnsureNonNativeArray(JavascriptArray *arr)
{
#if ENABLE_COPYONACCESS_ARRAY
JavascriptLibrary::CheckAndConvertCopyOnAccessNativeIntArray<Var>(arr);
#endif
if (JavascriptNativeIntArray::Is(arr))
{
arr = JavascriptNativeIntArray::ToVarArray((JavascriptNativeIntArray*)arr);
Expand Down Expand Up @@ -10931,23 +10995,6 @@ namespace Js
}
}

template<typename T>
void JavascriptArray::InternalFillFromPrototype(JavascriptArray *dstArray, const T& dstIndex, JavascriptArray *srcArray, uint32 start, uint32 end, uint32 count)
{
RecyclableObject* prototype = srcArray->GetPrototype();
while (start + count != end && JavascriptOperators::GetTypeId(prototype) != TypeIds_Null)
{
ForEachOwnMissingArrayIndexOfObject(srcArray, dstArray, prototype, start, end, dstIndex, [&](uint32 index, Var value) {
T n = dstIndex + (index - start);
dstArray->DirectSetItemAt(n, value);

count++;
});

prototype = prototype->GetPrototype();
}
}

Var JavascriptArray::SpreadArrayArgs(Var arrayToSpread, const Js::AuxArray<uint32> *spreadIndices, ScriptContext *scriptContext)
{
// At this stage we have an array literal with some arguments to be spread.
Expand Down Expand Up @@ -11458,6 +11505,11 @@ namespace Js
{
if (!JavascriptOperators::GetProperty((RecyclableObject*)constructor, PropertyIds::_symbolSpecies, &constructor, scriptContext))
{
if (pIsBuiltinArrayCtor != nullptr)
{
*pIsBuiltinArrayCtor = false;
}

return nullptr;
}
if (constructor == scriptContext->GetLibrary()->GetNull())
Expand Down
2 changes: 1 addition & 1 deletion lib/Runtime/Library/JavascriptPromise.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -910,7 +910,7 @@ namespace Js

try
{
values->DirectSetItemAt(index, x);
values->SetItem(index, x, PropertyOperation_None);
}
catch (JavascriptExceptionObject* e)
{
Expand Down
Loading

0 comments on commit c2787ef

Please sign in to comment.