Skip to content

Commit

Permalink
[CVE-2017-11797] Invalid memory read of out params on a bailout when …
Browse files Browse the repository at this point in the history 
…array destructuring is used as call arg

Destructuring will create try/catch/finally bytecode when emitting. This pattern can appear as an arg to the call node. We were emitting Argouts as when emit a
parameter, so it possible that we have try/catch/finally in between argouts.
The Finally optimization did not like the fact that Argouts are seperated. In order to fix we use temps to store those argouts temporarily and then later
those temps will be emitted as argouts.
Since we are emitting lots of temps, this change is done when we determine that args contain destructuring (We took parser help for that).
  • Loading branch information
@akroshg @agarwal-sandeep
akroshg authored and agarwal-sandeep committed Oct 10, 2017
1 parent 4e319aa commit 9a3ad7c
Show file tree
Hide file tree
Showing 4 changed files with 172 additions and 58 deletions.
10 changes: 9 additions & 1 deletion lib/Parser/Parse.cpp
View file
Expand Up @@ -90,6 +90,8 @@ Parser::Parser(Js::ScriptContext* scriptContext, BOOL strictMode, PageAllocator
m_stoppedDeferredParse = FALSE;
m_hasParallelJob = false;
m_doingFastScan = false;
m_isInParsingArgList = false;
m_hasDestructuringPattern = false;
m_scriptContext = scriptContext;
m_pCurrentAstSize = nullptr;
m_arrayDepth = 0;
Expand Down Expand Up @@ -1252,7 +1254,7 @@ Parser::CreateCallNode(OpCode nop, ParseNodePtr pnode1, ParseNodePtr pnode2,char
pnode->sxCall.isApplyCall = false;
pnode->sxCall.isEvalCall = false;
pnode->sxCall.isSuperCall = false;

pnode->sxCall.hasDestructuring = false;
pnode->ichMin = ichMin;
pnode->ichLim = ichLim;

Expand Down Expand Up @@ -3660,6 +3662,7 @@ ParseNodePtr Parser::ParsePostfixOperators(
{
case tkLParen:
{
AutoMarkInParsingArgs autoMarkInParsingArgs(this);
if (fInNew)
{
ParseNodePtr pnodeArgs = ParseArgList<buildAST>(&callOfConstants, &spreadArgCount, &count);
Expand All @@ -3672,6 +3675,8 @@ ParseNodePtr Parser::ParsePostfixOperators(
pnode->sxCall.isApplyCall = false;
pnode->sxCall.isEvalCall = false;
pnode->sxCall.isSuperCall = false;
pnode->sxCall.hasDestructuring = m_hasDestructuringPattern;
Assert(!m_hasDestructuringPattern || count > 0);
pnode->sxCall.argCount = count;
pnode->sxCall.spreadArgCount = spreadArgCount;
pnode->ichLim = m_pscan->IchLimTok();
Expand Down Expand Up @@ -3743,6 +3748,8 @@ ParseNodePtr Parser::ParsePostfixOperators(
pnode->sxCall.spreadArgCount = spreadArgCount;
pnode->sxCall.isApplyCall = false;
pnode->sxCall.isEvalCall = fCallIsEval;
pnode->sxCall.hasDestructuring = m_hasDestructuringPattern;
Assert(!m_hasDestructuringPattern || count > 0);
pnode->sxCall.argCount = count;
pnode->ichLim = m_pscan->IchLimTok();
}
Expand Down Expand Up @@ -8630,6 +8637,7 @@ ParseNodePtr Parser::ParseExpr(int oplMin,

if (buildAST)
{
this->SetHasDestructuringPattern(true);
pnode = ConvertToPattern(pnode);
}

Expand Down
33 changes: 32 additions & 1 deletion lib/Parser/Parse.h
View file
Expand Up @@ -136,6 +136,12 @@ class Parser
bool IsBackgroundParser() const { return m_isInBackground; }
bool IsDoingFastScan() const { return m_doingFastScan; }

bool GetIsInParsingArgList() const { return m_isInParsingArgList; }
void SetIsInParsingArgList(bool set) { m_isInParsingArgList = set; }

bool GetHasDestructuringPattern() const { return m_hasDestructuringPattern; }
void SetHasDestructuringPattern(bool set) { m_hasDestructuringPattern = set; }

static IdentPtr PidFromNode(ParseNodePtr pnode);

ParseNode* CopyPnode(ParseNode* pnode);
Expand Down Expand Up @@ -395,7 +401,8 @@ class Parser
bool m_isInBackground;
bool m_reparsingLambdaParams;
bool m_disallowImportExportStmt;

bool m_isInParsingArgList;
bool m_hasDestructuringPattern;
// This bool is used for deferring the shorthand initializer error ( {x = 1}) - as it is allowed in the destructuring grammar.
bool m_hasDeferredShorthandInitError;
uint * m_pnestedCount; // count of functions nested at one level below the current node
Expand Down Expand Up @@ -1110,6 +1117,30 @@ class Parser
}
};

class AutoMarkInParsingArgs
{
public:
AutoMarkInParsingArgs(Parser * parser)
: m_parser(parser)
{
m_prevState = m_parser->GetIsInParsingArgList();
m_parser->SetHasDestructuringPattern(false);
m_parser->SetIsInParsingArgList(true);
}
~AutoMarkInParsingArgs()
{
m_parser->SetIsInParsingArgList(m_prevState);
if (!m_prevState)
{
m_parser->SetHasDestructuringPattern(false);
}
}

private:
Parser *m_parser;
bool m_prevState;
};

public:
charcount_t GetSourceIchLim() { return m_sourceLim; }
static BOOL NodeEqualsName(ParseNodePtr pnode, LPCOLESTR sz, uint32 cch);
Expand Down
1 change: 1 addition & 0 deletions lib/Parser/ptree.h
View file
Expand Up @@ -459,6 +459,7 @@ struct PnCall
BYTE isApplyCall : 1;
BYTE isEvalCall : 1;
BYTE isSuperCall : 1;
BYTE hasDestructuring : 1;
};

struct PnStmt
Expand Down
186 changes: 130 additions & 56 deletions lib/Runtime/ByteCode/ByteCodeEmitter.cpp
View file
Expand Up @@ -6879,12 +6879,112 @@ void EmitList(ParseNode *pnode, ByteCodeGenerator *byteCodeGenerator, FuncInfo *
}
}

void EmitSpreadArgToListBytecodeInstr(ByteCodeGenerator *byteCodeGenerator, FuncInfo *funcInfo, Js::RegSlot argLoc, Js::ProfileId callSiteId, Js::ArgSlot &argIndex)
void EmitOneArg(
ParseNode *pnode,
BOOL fAssignRegs,
ByteCodeGenerator *byteCodeGenerator,
FuncInfo *funcInfo,
Js::ProfileId callSiteId,
Js::ArgSlot &argIndex,
Js::ArgSlot &spreadIndex,
Js::RegSlot argTempLocation,
Js::AuxArray<uint32> *spreadIndices = nullptr
)
{
Js::RegSlot regVal = funcInfo->AcquireTmpRegister();
byteCodeGenerator->Writer()->Reg2(Js::OpCode::LdCustomSpreadIteratorList, regVal, argLoc);
byteCodeGenerator->Writer()->ArgOut<true>(++argIndex, regVal, callSiteId);
funcInfo->ReleaseTmpRegister(regVal);
bool noArgOuts = argTempLocation != Js::Constants::NoRegister;

// If this is a put, the arguments have already been evaluated (see EmitReference).
// We just need to emit the ArgOut instructions.
if (fAssignRegs)
{
Emit(pnode, byteCodeGenerator, funcInfo, false);
}

if (pnode->nop == knopEllipsis)
{
Assert(spreadIndices != nullptr);
spreadIndices->elements[spreadIndex++] = argIndex + 1; // account for 'this'
Js::RegSlot regVal = funcInfo->AcquireTmpRegister();
byteCodeGenerator->Writer()->Reg2(Js::OpCode::LdCustomSpreadIteratorList, regVal, pnode->location);
if (noArgOuts)
{
byteCodeGenerator->Writer()->Reg2(Js::OpCode::Ld_A, argTempLocation, regVal);
}
else
{
byteCodeGenerator->Writer()->ArgOut<true>(argIndex + 1, regVal, callSiteId);
}
funcInfo->ReleaseTmpRegister(regVal);
}
else
{
if (noArgOuts)
{
byteCodeGenerator->Writer()->Reg2(Js::OpCode::Ld_A, argTempLocation, pnode->location);
}
else
{
byteCodeGenerator->Writer()->ArgOut<true>(argIndex + 1, pnode->location, callSiteId);
}
}
argIndex++;

if (fAssignRegs)
{
funcInfo->ReleaseLoc(pnode);
}
}

size_t EmitArgsWithArgOutsAtEnd(
ParseNode *pnode,
BOOL fAssignRegs,
ByteCodeGenerator *byteCodeGenerator,
FuncInfo *funcInfo,
Js::ProfileId callSiteId,
Js::RegSlot thisLocation,
Js::ArgSlot argsCountForStartCall,
Js::AuxArray<uint32> *spreadIndices = nullptr
)
{
AssertOrFailFast(pnode != nullptr);

Js::ArgSlot argIndex = 0;
Js::ArgSlot spreadIndex = 0;

Js::RegSlot argTempLocation = funcInfo->AcquireTmpRegister();
Js::RegSlot firstArgTempLocation = argTempLocation;

while (pnode->nop == knopList)
{
EmitOneArg(pnode->sxBin.pnode1, fAssignRegs, byteCodeGenerator, funcInfo, callSiteId, argIndex, spreadIndex, argTempLocation, spreadIndices);
pnode = pnode->sxBin.pnode2;
argTempLocation = funcInfo->AcquireTmpRegister();
}

EmitOneArg(pnode, fAssignRegs, byteCodeGenerator, funcInfo, callSiteId, argIndex, spreadIndex, argTempLocation, spreadIndices);

byteCodeGenerator->Writer()->StartCall(Js::OpCode::StartCall, argsCountForStartCall);

// Emit all argOuts now

if (thisLocation != Js::Constants::NoRegister)
{
// Emit the "this" object.
byteCodeGenerator->Writer()->ArgOut<true>(0, thisLocation, callSiteId);
}

for (Js::ArgSlot index = 0; index < argIndex; index++)
{
byteCodeGenerator->Writer()->ArgOut<true>(index + 1, firstArgTempLocation + index, callSiteId);
}

// Now release all those temps register
for (Js::ArgSlot index = argIndex; index > 0; index--)
{
funcInfo->ReleaseTmpRegister(argTempLocation--);
}

return argIndex;
}

size_t EmitArgs(
Expand All @@ -6903,57 +7003,18 @@ size_t EmitArgs(
{
while (pnode->nop == knopList)
{
// If this is a put, the arguments have already been evaluated (see EmitReference).
// We just need to emit the ArgOut instructions.
if (fAssignRegs)
{
Emit(pnode->sxBin.pnode1, byteCodeGenerator, funcInfo, false);
}

if (pnode->sxBin.pnode1->nop == knopEllipsis)
{
Assert(spreadIndices != nullptr);
spreadIndices->elements[spreadIndex++] = argIndex + 1; // account for 'this'
EmitSpreadArgToListBytecodeInstr(byteCodeGenerator, funcInfo, pnode->sxBin.pnode1->location, callSiteId, argIndex);
}
else
{
byteCodeGenerator->Writer()->ArgOut<true>(++argIndex, pnode->sxBin.pnode1->location, callSiteId);
}
if (fAssignRegs)
{
funcInfo->ReleaseLoc(pnode->sxBin.pnode1);
}

EmitOneArg(pnode->sxBin.pnode1, fAssignRegs, byteCodeGenerator, funcInfo, callSiteId, argIndex, spreadIndex, Js::Constants::NoRegister, spreadIndices);
pnode = pnode->sxBin.pnode2;
}

// If this is a put, the call target has already been evaluated (see EmitReference).
if (fAssignRegs)
{
Emit(pnode, byteCodeGenerator, funcInfo, false);
}

if (pnode->nop == knopEllipsis)
{
Assert(spreadIndices != nullptr);
spreadIndices->elements[spreadIndex++] = argIndex + 1; // account for 'this'
EmitSpreadArgToListBytecodeInstr(byteCodeGenerator, funcInfo, pnode->location, callSiteId, argIndex);
}
else
{
byteCodeGenerator->Writer()->ArgOut<true>(++argIndex, pnode->location, callSiteId);
}

if (fAssignRegs)
{
funcInfo->ReleaseLoc(pnode);
}
EmitOneArg(pnode, fAssignRegs, byteCodeGenerator, funcInfo, callSiteId, argIndex, spreadIndex, Js::Constants::NoRegister, spreadIndices);
}

return argIndex;
}



void EmitArgListStart(
Js::RegSlot thisLocation,
ByteCodeGenerator *byteCodeGenerator,
Expand Down Expand Up @@ -7074,13 +7135,18 @@ Js::ArgSlot EmitArgList(
ByteCodeGenerator *byteCodeGenerator,
FuncInfo *funcInfo,
Js::ProfileId callSiteId,
Js::ArgSlot argsCountForStartCall,
bool emitArgOutsAtEnd,
uint16 spreadArgCount = 0,
Js::AuxArray<uint32> **spreadIndices = nullptr)
{
// This function emits the arguments for a call.
// ArgOut's with uses immediately following defs.

EmitArgListStart(thisLocation, byteCodeGenerator, funcInfo, callSiteId);
if (!emitArgOutsAtEnd)
{
byteCodeGenerator->Writer()->StartCall(Js::OpCode::StartCall, argsCountForStartCall);
EmitArgListStart(thisLocation, byteCodeGenerator, funcInfo, callSiteId);
}

Js::RegSlot evalLocation = Js::Constants::NoRegister;

Expand All @@ -7100,7 +7166,15 @@ Js::ArgSlot EmitArgList(
*spreadIndices = AnewPlus(byteCodeGenerator->GetAllocator(), extraAlloc, Js::AuxArray<uint32>, spreadArgCount);
}

size_t argIndex = EmitArgs(pnode, fAssignRegs, byteCodeGenerator, funcInfo, callSiteId, spreadIndices == nullptr ? nullptr : *spreadIndices);
size_t argIndex = 0;
if (emitArgOutsAtEnd)
{
argIndex = EmitArgsWithArgOutsAtEnd(pnode, fAssignRegs, byteCodeGenerator, funcInfo, callSiteId, thisLocation, argsCountForStartCall, spreadIndices == nullptr ? nullptr : *spreadIndices);
}
else
{
argIndex = EmitArgs(pnode, fAssignRegs, byteCodeGenerator, funcInfo, callSiteId, spreadIndices == nullptr ? nullptr : *spreadIndices);
}

Js::ArgSlot argumentsCount = EmitArgListEnd(pnode, rhsLocation, thisLocation, evalLocation, newTargetLocation, byteCodeGenerator, funcInfo, argIndex, callSiteId);

Expand Down Expand Up @@ -7855,11 +7929,12 @@ void EmitNew(ParseNode* pnode, ByteCodeGenerator* byteCodeGenerator, FuncInfo* f
}
else
{
byteCodeGenerator->Writer()->StartCall(Js::OpCode::StartCall, argCount);

uint32 actualArgCount = 0;

if (IsCallOfConstants(pnode))
{
byteCodeGenerator->Writer()->StartCall(Js::OpCode::StartCall, argCount);
funcInfo->ReleaseLoc(pnode->sxCall.pnodeTarget);
actualArgCount = EmitNewObjectOfConstants(pnode, byteCodeGenerator, funcInfo, argCount);
}
Expand All @@ -7880,10 +7955,9 @@ void EmitNew(ParseNode* pnode, ByteCodeGenerator* byteCodeGenerator, FuncInfo* f

Js::AuxArray<uint32> *spreadIndices = nullptr;
actualArgCount = EmitArgList(pnode->sxCall.pnodeArgs, Js::Constants::NoRegister, Js::Constants::NoRegister, Js::Constants::NoRegister,
false, true, byteCodeGenerator, funcInfo, callSiteId, pnode->sxCall.spreadArgCount, &spreadIndices);
false, true, byteCodeGenerator, funcInfo, callSiteId, argCount, pnode->sxCall.hasDestructuring, pnode->sxCall.spreadArgCount, &spreadIndices);
funcInfo->ReleaseLoc(pnode->sxCall.pnodeTarget);


if (pnode->sxCall.spreadArgCount > 0)
{
Assert(spreadIndices != nullptr);
Expand Down Expand Up @@ -8025,9 +8099,9 @@ void EmitCall(

Js::ProfileId callSiteId = byteCodeGenerator->GetNextCallSiteId(Js::OpCode::CallI);

byteCodeGenerator->Writer()->StartCall(Js::OpCode::StartCall, argSlotCount);
Js::AuxArray<uint32> *spreadIndices;
Js::ArgSlot actualArgCount = EmitArgList(pnodeArgs, rhsLocation, thisLocation, newTargetLocation, fIsEval, fEvaluateComponents, byteCodeGenerator, funcInfo, callSiteId, spreadArgCount, &spreadIndices);
Js::ArgSlot actualArgCount = EmitArgList(pnodeArgs, rhsLocation, thisLocation, newTargetLocation, fIsEval, fEvaluateComponents, byteCodeGenerator, funcInfo, callSiteId, argSlotCount, pnode->sxCall.hasDestructuring, spreadArgCount, &spreadIndices);

Assert(argSlotCount == actualArgCount);

if (!fEvaluateComponents)
Expand Down

0 comments on commit 9a3ad7c

Please sign in to comment.