Skip to content

Commit

Permalink
[CVE-2019-0923] Scripting Engine Memory Corruption - Individual
Browse files Browse the repository at this point in the history
After reparsing the coroutines (generator/async functions) the register which holds yield data has changed.
This is due to scoping structure for debugger.
In order to fix that allow the same scoping numbering for coroutine as well.
Also added a failfast to see if the yield register information is the same before and after reparsing.
  • Loading branch information
akroshg authored and MikeHolman committed May 13, 2019
1 parent d85b502 commit 9725847
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 7 deletions.
1 change: 1 addition & 0 deletions lib/Runtime/ByteCode/ByteCodeEmitter.cpp
Expand Up @@ -3590,6 +3590,7 @@ void ByteCodeGenerator::StartEmitFunction(ParseNodeFnc *pnodeFnc)
#if ENABLE_TTD
&& !funcInfo->GetParsedFunctionBody()->GetScriptContext()->GetThreadContext()->IsRuntimeInTTDMode()
#endif
&& !funcInfo->byteCodeFunction->IsCoroutine()
);

if (funcInfo->GetHasCachedScope())
Expand Down
12 changes: 6 additions & 6 deletions lib/Runtime/ByteCode/ByteCodeGenerator.cpp
Expand Up @@ -119,10 +119,10 @@ void EndVisitBlock(ParseNodeBlock *pnode, ByteCodeGenerator *byteCodeGenerator)
Scope *scope = pnode->scope;
FuncInfo *func = scope->GetFunc();

if (!byteCodeGenerator->IsInDebugMode() &&
scope->HasInnerScopeIndex())
if (!(byteCodeGenerator->IsInDebugMode() || func->byteCodeFunction->IsCoroutine())
&& scope->HasInnerScopeIndex())
{
// In debug mode, don't release the current index, as we're giving each scope a unique index, regardless
// In debug mode (or for the generator/async function), don't release the current index, as we're giving each scope a unique index, regardless
// of nesting.
Assert(scope->GetInnerScopeIndex() == func->CurrentInnerScopeIndex());
func->ReleaseInnerScopeIndex();
Expand Down Expand Up @@ -155,12 +155,12 @@ void BeginVisitCatch(ParseNode *pnode, ByteCodeGenerator *byteCodeGenerator)
void EndVisitCatch(ParseNode *pnode, ByteCodeGenerator *byteCodeGenerator)
{
Scope *scope = pnode->AsParseNodeCatch()->scope;
FuncInfo *func = scope->GetFunc();

if (scope->HasInnerScopeIndex() && !byteCodeGenerator->IsInDebugMode())
if (scope->HasInnerScopeIndex() && !(byteCodeGenerator->IsInDebugMode() || func->byteCodeFunction->IsCoroutine()))
{
// In debug mode, don't release the current index, as we're giving each scope a unique index,
// In debug mode (or for the generator/async function), don't release the current index, as we're giving each scope a unique index,
// regardless of nesting.
FuncInfo *func = scope->GetFunc();

Assert(scope->GetInnerScopeIndex() == func->CurrentInnerScopeIndex());
func->ReleaseInnerScopeIndex();
Expand Down
34 changes: 33 additions & 1 deletion lib/Runtime/Debug/DebugContext.cpp
Expand Up @@ -193,6 +193,11 @@ namespace Js
Js::TempArenaAllocatorObject *tempAllocator = nullptr;
JsUtil::List<Js::FunctionInfo *, Recycler>* pFunctionsToRegister = nullptr;
JsUtil::List<Js::Utf8SourceInfo *, Recycler, false, Js::CopyRemovePolicy, RecyclerPointerComparer>* utf8SourceInfoList = nullptr;
typedef JsUtil::BaseDictionary<uint32, RegSlot, ArenaAllocator, PowerOf2SizePolicy> FunctionStartToYieldRegister;

// This container ensures that for Generator/Async functions the yield register is same between non-debug to debug parse.
// Each entry represent a function's start position (each function will have unique start position in a file) and that function yield register
FunctionStartToYieldRegister *yieldFunctions = nullptr;

HRESULT hr = S_OK;
ThreadContext* threadContext = this->scriptContext->GetThreadContext();
Expand All @@ -201,6 +206,7 @@ namespace Js
tempAllocator = threadContext->GetTemporaryAllocator(_u("debuggerAlloc"));

utf8SourceInfoList = JsUtil::List<Js::Utf8SourceInfo *, Recycler, false, Js::CopyRemovePolicy, RecyclerPointerComparer>::New(this->scriptContext->GetRecycler());
yieldFunctions = Anew(tempAllocator->GetAllocator(), FunctionStartToYieldRegister, tempAllocator->GetAllocator());

this->MapUTF8SourceInfoUntil([&](Js::Utf8SourceInfo * sourceInfo) -> bool
{
Expand Down Expand Up @@ -276,6 +282,23 @@ namespace Js
this->hostDebugContext->SetThreadDescription(sourceInfo->GetSourceContextInfo()->url); // the HRESULT is omitted.
}

if (shouldReparseFunctions)
{
yieldFunctions->Clear();
BEGIN_TRANSLATE_OOM_TO_HRESULT_NESTED
{
sourceInfo->MapFunction([&](Js::FunctionBody *const pFuncBody)
{
if (pFuncBody->IsCoroutine() && pFuncBody->GetYieldRegister() != Js::Constants::NoRegister)
{
yieldFunctions->Add(pFuncBody->StartInDocument(), pFuncBody->GetYieldRegister());
}
});
}
END_TRANSLATE_OOM_TO_HRESULT(hr);
DEBUGGER_ATTACHDETACH_FATAL_ERROR_IF_FAILED(hr);
}

bool fHasDoneSourceRundown = false;
for (int i = 0; i < pFunctionsToRegister->Count(); i++)
{
Expand Down Expand Up @@ -326,8 +349,17 @@ namespace Js

if (shouldReparseFunctions)
{
sourceInfo->MapFunction([](Js::FunctionBody *const pFuncBody)
sourceInfo->MapFunction([&](Js::FunctionBody *const pFuncBody)
{
if (pFuncBody->IsCoroutine())
{
RegSlot oldYieldRegister = Constants::NoRegister;
if (yieldFunctions->TryGetValue(pFuncBody->StartInDocument(), &oldYieldRegister))
{
AssertOrFailFast(pFuncBody->GetYieldRegister() == oldYieldRegister);
}
}

if (pFuncBody->IsFunctionParsed())
{
pFuncBody->ReinitializeExecutionModeAndLimits();
Expand Down

0 comments on commit 9725847

Please sign in to comment.