Permalink
Browse files

[CVE-2018-0776] JIT: stack-to-heap copy bug - Google, Inc.

This change fixes a type-confusion bug that can occur with Native arrays allocated on the stack. Once JIT'd code expects a Native array to be used on the stack, the POC converts it to a Var array. This is combined with current behavior of the Arguments property, which moves the array from the stack to the heap. The result of these two assumptions is natively setting a Float value where a Var value is expected, letting any arbitrary floating-point number be written to memory and subsequently accessed as a Var.

This fix forces a deep copy of Arrays that are returned via Arguments. This ensures that the new object created points to its own buffers. This also indicates a divergence with the original object and the one created by Arguments; however, there is currently no standard to define this behavior.
  • Loading branch information...
Thomas Moore (CHAKRA)
Thomas Moore (CHAKRA) committed Dec 13, 2017
1 parent 985a82f commit 40e45fc38189cc021267c65d42ca2fb5f899f9de
@@ -1006,7 +1006,7 @@ BailOutRecord::RestoreValue(IR::BailOutKind bailOutKind, Js::JavascriptCallStack
if (boxStackInstance)
{
Js::Var oldValue = value;
value = Js::JavascriptOperators::BoxStackInstance(oldValue, scriptContext, /* allowStackFunction */ true);
value = Js::JavascriptOperators::BoxStackInstance(oldValue, scriptContext, /* allowStackFunction */ true, /* deepCopy */ false);

if (oldValue != value)
{
@@ -1275,7 +1275,7 @@ BailOutRecord::BailOutInlinedHelper(Js::JavascriptCallStackLayout * layout, Bail
if (inlineeFrameRecord)
{
InlinedFrameLayout* outerMostFrame = (InlinedFrameLayout *)(((uint8 *)Js::JavascriptCallStackLayout::ToFramePointer(layout)) - entryPointInfo->frameHeight);
inlineeFrameRecord->RestoreFrames(functionBody, outerMostFrame, layout);
inlineeFrameRecord->RestoreFrames(functionBody, outerMostFrame, layout, false /* deepCopy */);
}
}

@@ -1480,7 +1480,7 @@ BailOutRecord::BailOutHelper(Js::JavascriptCallStackLayout * layout, Js::ScriptF
{
const Js::Var arg = args.Values[i];
BAILOUT_VERBOSE_TRACE(executeFunction, bailOutKind, _u("BailOut: Argument #%3u: value: 0x%p"), i, arg);
const Js::Var boxedArg = Js::JavascriptOperators::BoxStackInstance(arg, functionScriptContext, true);
const Js::Var boxedArg = Js::JavascriptOperators::BoxStackInstance(arg, functionScriptContext, /* allowStackFunction */ true, /* deepCopy */ false);
if(boxedArg != arg)
{
args.Values[i] = boxedArg;
@@ -1775,7 +1775,7 @@ BailOutRecord::BailOutHelper(Js::JavascriptCallStackLayout * layout, Js::ScriptF
aReturn = Js::JavascriptFunction::FinishConstructor(aReturn, args.Values[0], function);

Js::Var oldValue = aReturn;
aReturn = Js::JavascriptOperators::BoxStackInstance(oldValue, functionScriptContext, /* allowStackFunction */ true);
aReturn = Js::JavascriptOperators::BoxStackInstance(oldValue, functionScriptContext, /* allowStackFunction */ true, /* deepCopy */ false);
#if ENABLE_DEBUG_CONFIG_OPTIONS
if (oldValue != aReturn)
{
@@ -199,13 +199,14 @@ void InlineeFrameRecord::Finalize(Func* inlinee, uint32 currentOffset)
Assert(this->inlineDepth != 0);
}

void InlineeFrameRecord::Restore(Js::FunctionBody* functionBody, InlinedFrameLayout *inlinedFrame, Js::JavascriptCallStackLayout * layout) const
void InlineeFrameRecord::Restore(Js::FunctionBody* functionBody, InlinedFrameLayout *inlinedFrame, Js::JavascriptCallStackLayout * layout, bool deepCopy) const
{
Assert(this->inlineDepth != 0);
Assert(inlineeStartOffset != 0);

BAILOUT_VERBOSE_TRACE(functionBody, _u("Restore function object: "));
Js::Var varFunction = this->Restore(this->functionOffset, /*isFloat64*/ false, /*isInt32*/ false, layout, functionBody);
// No deepCopy needed for just the function
Js::Var varFunction = this->Restore(this->functionOffset, /*isFloat64*/ false, /*isInt32*/ false, layout, functionBody, /*deepCopy*/ false);
Assert(Js::ScriptFunction::Is(varFunction));

Js::ScriptFunction* function = Js::ScriptFunction::FromVar(varFunction);
@@ -219,7 +220,9 @@ void InlineeFrameRecord::Restore(Js::FunctionBody* functionBody, InlinedFrameLay
bool isInt32 = losslessInt32Args.Test(i) != 0;
BAILOUT_VERBOSE_TRACE(functionBody, _u("Restore argument %d: "), i);

Js::Var var = this->Restore(this->argOffsets[i], isFloat64, isInt32, layout, functionBody);
// Forward deepCopy flag for the arguments in case their data must be guaranteed
// to have its own lifetime
Js::Var var = this->Restore(this->argOffsets[i], isFloat64, isInt32, layout, functionBody, deepCopy);
#if DBG
if (!Js::TaggedNumber::Is(var))
{
@@ -233,7 +236,7 @@ void InlineeFrameRecord::Restore(Js::FunctionBody* functionBody, InlinedFrameLay
BAILOUT_FLUSH(functionBody);
}

void InlineeFrameRecord::RestoreFrames(Js::FunctionBody* functionBody, InlinedFrameLayout* outerMostFrame, Js::JavascriptCallStackLayout* callstack)
void InlineeFrameRecord::RestoreFrames(Js::FunctionBody* functionBody, InlinedFrameLayout* outerMostFrame, Js::JavascriptCallStackLayout* callstack, bool deepCopy)
{
InlineeFrameRecord* innerMostRecord = this;
class AutoReverse
@@ -271,7 +274,7 @@ void InlineeFrameRecord::RestoreFrames(Js::FunctionBody* functionBody, InlinedFr

while (currentRecord)
{
currentRecord->Restore(functionBody, currentFrame, callstack);
currentRecord->Restore(functionBody, currentFrame, callstack, deepCopy);
currentRecord = currentRecord->parent;
currentFrame = currentFrame->Next();
}
@@ -280,7 +283,7 @@ void InlineeFrameRecord::RestoreFrames(Js::FunctionBody* functionBody, InlinedFr
currentFrame->callInfo.Count = 0;
}

Js::Var InlineeFrameRecord::Restore(int offset, bool isFloat64, bool isInt32, Js::JavascriptCallStackLayout * layout, Js::FunctionBody* functionBody) const
Js::Var InlineeFrameRecord::Restore(int offset, bool isFloat64, bool isInt32, Js::JavascriptCallStackLayout * layout, Js::FunctionBody* functionBody, bool deepCopy) const
{
Js::Var value;
bool boxStackInstance = true;
@@ -322,7 +325,7 @@ Js::Var InlineeFrameRecord::Restore(int offset, bool isFloat64, bool isInt32, Js
if (boxStackInstance)
{
Js::Var oldValue = value;
value = Js::JavascriptOperators::BoxStackInstance(oldValue, functionBody->GetScriptContext(), /* allowStackFunction */ true);
value = Js::JavascriptOperators::BoxStackInstance(oldValue, functionBody->GetScriptContext(), /* allowStackFunction */ true, deepCopy);

#if ENABLE_DEBUG_CONFIG_OPTIONS
if (oldValue != value)
@@ -108,7 +108,7 @@ struct InlineeFrameRecord
}

void PopulateParent(Func* func);
void RestoreFrames(Js::FunctionBody* functionBody, InlinedFrameLayout* outerMostInlinee, Js::JavascriptCallStackLayout* callstack);
void RestoreFrames(Js::FunctionBody* functionBody, InlinedFrameLayout* outerMostInlinee, Js::JavascriptCallStackLayout* callstack, bool deepCopy);
void Finalize(Func* inlinee, uint currentOffset);
#if DBG_DUMP
void Dump() const;
@@ -123,8 +123,8 @@ struct InlineeFrameRecord
}

private:
void Restore(Js::FunctionBody* functionBody, InlinedFrameLayout *outerMostFrame, Js::JavascriptCallStackLayout * layout) const;
Js::Var Restore(int offset, bool isFloat64, bool isInt32, Js::JavascriptCallStackLayout * layout, Js::FunctionBody* functionBody) const;
void Restore(Js::FunctionBody* functionBody, InlinedFrameLayout *outerMostFrame, Js::JavascriptCallStackLayout * layout, bool deepCopy) const;
Js::Var Restore(int offset, bool isFloat64, bool isInt32, Js::JavascriptCallStackLayout * layout, Js::FunctionBody* functionBody, bool deepCopy) const;
InlineeFrameRecord* Reverse();
};

@@ -370,7 +370,7 @@ namespace Js
{
activeScopeObject->SetPropertyWithAttributes(
resolveObject.propId,
JavascriptOperators::BoxStackInstance(resolveObject.obj, scriptContext), //The value escapes, box if necessary.
JavascriptOperators::BoxStackInstance(resolveObject.obj, scriptContext, /* allowStackFunction */ false, /* deepCopy */ false), //The value escapes, box if necessary.
resolveObject.isConst ? PropertyConstDefaults : PropertyDynamicTypeDefaults,
nullptr);
}
@@ -348,7 +348,7 @@ namespace Js
{
activeScopeObject->SetPropertyWithAttributes(
Js::PropertyIds::_this,
JavascriptOperators::BoxStackInstance(varThis, scriptContext), //The value escapes, box if necessary.
JavascriptOperators::BoxStackInstance(varThis, scriptContext, /* allowStackFunction */ false, /* deepCopy */ false), //The value escapes, box if necessary.
PropertyConstDefaults,
nullptr);
#if DBG
@@ -9790,7 +9790,7 @@ namespace Js
}

Js::Var
JavascriptOperators::BoxStackInstance(Js::Var instance, ScriptContext * scriptContext, bool allowStackFunction)
JavascriptOperators::BoxStackInstance(Js::Var instance, ScriptContext * scriptContext, bool allowStackFunction, bool deepCopy)
{
if (!ThreadContext::IsOnStack(instance) || (allowStackFunction && !TaggedNumber::Is(instance) && (*(int*)instance & 1)))
{
@@ -9812,11 +9812,11 @@ namespace Js
case Js::TypeIds_Object:
return DynamicObject::BoxStackInstance(DynamicObject::FromVar(instance));
case Js::TypeIds_Array:
return JavascriptArray::BoxStackInstance(JavascriptArray::FromVar(instance));
return JavascriptArray::BoxStackInstance(JavascriptArray::FromVar(instance), deepCopy);
case Js::TypeIds_NativeIntArray:
return JavascriptNativeIntArray::BoxStackInstance(JavascriptNativeIntArray::FromVar(instance));
return JavascriptNativeIntArray::BoxStackInstance(JavascriptNativeIntArray::FromVar(instance), deepCopy);
case Js::TypeIds_NativeFloatArray:
return JavascriptNativeFloatArray::BoxStackInstance(JavascriptNativeFloatArray::FromVar(instance));
return JavascriptNativeFloatArray::BoxStackInstance(JavascriptNativeFloatArray::FromVar(instance), deepCopy);
case Js::TypeIds_Function:
Assert(allowStackFunction);
// Stack functions are deal with not mar mark them, but by nested function escape analysis
@@ -611,7 +611,7 @@ namespace Js
#endif
static RecyclableObject *GetCallableObjectOrThrow(const Var callee, ScriptContext *const scriptContext);

static Js::Var BoxStackInstance(Js::Var value, ScriptContext * scriptContext, bool allowStackFunction = false);
static Js::Var BoxStackInstance(Js::Var value, ScriptContext * scriptContext, bool allowStackFunction, bool deepCopy);
static BOOL PropertyReferenceWalkUnscopable(Var instance, RecyclableObject** propertyObject, PropertyId propertyId, Var* value, PropertyValueInfo* info, ScriptContext* requestContext);
static BOOL PropertyReferenceWalk(Var instance, RecyclableObject** propertyObject, PropertyId propertyId, Var* value, PropertyValueInfo* info, ScriptContext* requestContext);

@@ -450,7 +450,7 @@ namespace Js
// are inlined frames on the stack the InlineeCallInfo of the first inlined frame
// has the native offset of the current physical frame.
Assert(!*inlinee);
InlinedFrameWalker::FromPhysicalFrame(tmpFrameWalker, currentFrame, ScriptFunction::FromVar(parentFunction), PreviousInterpreterFrameIsFromBailout(), loopNum, this, useInternalFrameInfo);
InlinedFrameWalker::FromPhysicalFrame(tmpFrameWalker, currentFrame, ScriptFunction::FromVar(parentFunction), PreviousInterpreterFrameIsFromBailout(), loopNum, this, useInternalFrameInfo, false /*noAlloc*/, false /*deepCopy*/);
inlineeOffset = tmpFrameWalker.GetBottomMostInlineeOffset();
tmpFrameWalker.Close();
}
@@ -555,7 +555,8 @@ namespace Js
}

bool hasInlinedFramesOnStack = InlinedFrameWalker::FromPhysicalFrame(inlinedFrameWalker, currentFrame,
ScriptFunction::FromVar(function), true /*fromBailout*/, loopNum, this, false /*useInternalFrameInfo*/);
ScriptFunction::FromVar(function), true /*fromBailout*/, loopNum, this, false /*useInternalFrameInfo*/, false /*noAlloc*/, this->deepCopyForArgs);

if (hasInlinedFramesOnStack)
{
// We're now back in the state where currentFrame == physical frame of the inliner, but
@@ -602,7 +603,18 @@ namespace Js
// Check whether there are inlined frames nested in this native frame. The corresponding check for
// a jitted loop body frame should have been done in CheckJavascriptFrame
Assert(lastInternalFrameInfo.codeAddress == nullptr);
if (InlinedFrameWalker::FromPhysicalFrame(inlinedFrameWalker, currentFrame, ScriptFunction::FromVar(function)))
bool inlinedFramesFound = InlinedFrameWalker::FromPhysicalFrame(
inlinedFrameWalker,
currentFrame,
ScriptFunction::FromVar(function),
false, // fromBailout
-1, // loopNum
nullptr,// walker
false, // useInternalFrameInfo
false, // noAlloc
this->deepCopyForArgs
);
if (inlinedFramesFound)
{
this->inlinedFramesBeingWalked = inlinedFrameWalker.Next(inlinedFrameCallInfo);
this->hasInlinedFramesOnStack = true;
@@ -621,7 +633,8 @@ namespace Js
_NOINLINE
JavascriptStackWalker::JavascriptStackWalker(ScriptContext * scriptContext, bool useEERContext, PVOID returnAddress, bool _forceFullWalk /*=false*/) :
inlinedFrameCallInfo(CallFlags_None, 0), shouldDetectPartiallyInitializedInterpreterFrame(true), forceFullWalk(_forceFullWalk),
previousInterpreterFrameIsFromBailout(false), previousInterpreterFrameIsForLoopBody(false), hasInlinedFramesOnStack(false)
previousInterpreterFrameIsFromBailout(false), previousInterpreterFrameIsForLoopBody(false), hasInlinedFramesOnStack(false),
deepCopyForArgs(false)
{
if (scriptContext == NULL)
{
@@ -917,7 +930,7 @@ namespace Js
Assert((this->interpreterFrame->GetFlags() & Js::InterpreterStackFrameFlags_FromBailOut) != 0);
InlinedFrameWalker tmpFrameWalker;
Assert(InlinedFrameWalker::FromPhysicalFrame(tmpFrameWalker, currentFrame, Js::ScriptFunction::FromVar(argv[JavascriptFunctionArgIndex_Function]),
true /*fromBailout*/, this->tempInterpreterFrame->GetCurrentLoopNum(), this, false /*useInternalFrameInfo*/, true /*noAlloc*/));
true /*fromBailout*/, this->tempInterpreterFrame->GetCurrentLoopNum(), this, false /*useInternalFrameInfo*/, true /*noAlloc*/, false /*deepCopy*/));
tmpFrameWalker.Close();
}
#endif //DBG
@@ -964,9 +977,10 @@ namespace Js
{
if (includeInlineFrames &&
InlinedFrameWalker::FromPhysicalFrame(inlinedFrameWalker, currentFrame, Js::ScriptFunction::FromVar(argv[JavascriptFunctionArgIndex_Function]),
false /*fromBailout*/, this->tempInterpreterFrame->GetCurrentLoopNum(), this, false /*useInternalFrameInfo*/))
false /*fromBailout*/, this->tempInterpreterFrame->GetCurrentLoopNum(), this, false /*useInternalFrameInfo*/, false /*noAlloc*/, this->deepCopyForArgs))
{
// Found inlined frames in a jitted loop body. We dont want to skip the inlined frames; walk all of them before setting codeAddress on lastInternalFrameInfo.
// DeepCopy here because, if there is an inlinee in a loop body, FromPhysicalFrame won't be called from UpdateFrame
this->inlinedFramesBeingWalked = inlinedFrameWalker.Next(inlinedFrameCallInfo);
this->hasInlinedFramesOnStack = true;
Assert(inlinedFramesBeingWalked);
@@ -1208,7 +1222,7 @@ namespace Js

#if ENABLE_NATIVE_CODEGEN
bool InlinedFrameWalker::FromPhysicalFrame(InlinedFrameWalker& self, StackFrame& physicalFrame, Js::ScriptFunction *parent, bool fromBailout,
int loopNum, const JavascriptStackWalker * const stackWalker, bool useInternalFrameInfo, bool noAlloc)
int loopNum, const JavascriptStackWalker * const stackWalker, bool useInternalFrameInfo, bool noAlloc, bool deepCopy)
{
bool inlinedFramesFound = false;
FunctionBody* parentFunctionBody = parent->GetFunctionBody();
@@ -1261,7 +1275,7 @@ namespace Js

if (record)
{
record->RestoreFrames(parent->GetFunctionBody(), outerMostFrame, JavascriptCallStackLayout::FromFramePointer(framePointer));
record->RestoreFrames(parent->GetFunctionBody(), outerMostFrame, JavascriptCallStackLayout::FromFramePointer(framePointer), deepCopy);
}
}

@@ -1347,7 +1361,7 @@ namespace Js

for (size_t i = 0; i < argCount; i++)
{
args[i] = Js::JavascriptOperators::BoxStackInstance(args[i], scriptContext);
args[i] = Js::JavascriptOperators::BoxStackInstance(args[i], scriptContext, false /*allowStackFunction*/, false /*deepCopy*/);
}
}

@@ -96,8 +96,8 @@ namespace Js
Assert(currentIndex == -1);
}

static bool FromPhysicalFrame(InlinedFrameWalker& self, StackFrame& physicalFrame, Js::ScriptFunction *parent, bool fromBailout = false,
int loopNum = -1, const JavascriptStackWalker * const walker = nullptr, bool useInternalFrameInfo = false, bool noAlloc = false);
static bool FromPhysicalFrame(InlinedFrameWalker& self, StackFrame& physicalFrame, Js::ScriptFunction *parent, bool fromBailout,
int loopNum, const JavascriptStackWalker * const walker, bool useInternalFrameInfo, bool noAlloc, bool deepCopy);
void Close();
bool Next(CallInfo& callInfo);
size_t GetArgc() const;
@@ -304,6 +304,8 @@ namespace Js
{
return previousInterpreterFrameIsFromBailout;
}

void SetDeepCopyForArguments() { deepCopyForArgs = true; }
#if DBG
static bool ValidateTopJitFrame(Js::ScriptContext* scriptContext);
#endif
@@ -328,6 +330,7 @@ namespace Js
bool previousInterpreterFrameIsFromBailout : 1;
bool previousInterpreterFrameIsForLoopBody : 1;
bool forceFullWalk : 1; // ignoring hasCaller
bool deepCopyForArgs : 1; // indicates when Var's data should be deep-copied when gathering Arguments for the frame

Var GetThisFromFrame() const; // returns 'this' object from the physical frame
Var GetCurrentArgumentsObject() const; // returns arguments object from the current frame, which may be virtual (belonging to an inlinee)
Oops, something went wrong.

0 comments on commit 40e45fc

Please sign in to comment.