[CVE-2018-8380] Edge - Incorrect callinfo on inlineeFrame on an exception - Google, Inc

lib/Runtime/Base/ThreadContext.cpp

@@ -124,7 +124,7 @@ ThreadContext::ThreadContext(AllocationPolicyManager * allocationPolicyManager,
     entryExitRecord(nullptr),
     leafInterpreterFrame(nullptr),
     threadServiceWrapper(nullptr),
-    tryCatchFrameAddr(nullptr),
+    tryHandlerAddrOfReturnAddr(nullptr),
     temporaryArenaAllocatorCount(0),
     temporaryGuestArenaAllocatorCount(0),
     crefSContextForDiag(0),

lib/Runtime/Base/ThreadContext.h

@@ -667,7 +667,7 @@ private:
     ThreadServiceWrapper* threadServiceWrapper;
     uint functionCount;
     uint sourceInfoCount;
-    void * tryCatchFrameAddr;
+    void * tryHandlerAddrOfReturnAddr;
     enum RedeferralState
     {
         InitialRedeferralState,
@@ -1268,8 +1268,8 @@ public:
     uint EnterScriptStart(Js::ScriptEntryExitRecord *, bool doCleanup);
     void EnterScriptEnd(Js::ScriptEntryExitRecord *, bool doCleanup);
 
-    void * GetTryCatchFrameAddr() { return this->tryCatchFrameAddr; }
-    void SetTryCatchFrameAddr(void * frameAddr) { this->tryCatchFrameAddr = frameAddr; }
+    void * GetTryHandlerAddrOfReturnAddr() { return this->tryHandlerAddrOfReturnAddr; }
+    void SetTryHandlerAddrOfReturnAddr(void * addrOfReturnAddr) { this->tryHandlerAddrOfReturnAddr = addrOfReturnAddr; }
 
     template <bool leaveForHost>
     void LeaveScriptStart(void *);

lib/Runtime/Language/InterpreterStackFrame.cpp

@@ -6492,6 +6492,8 @@ skipThunk:
             this->OrFlags(InterpreterStackFrameFlags_WithinTryBlock);
 
             Js::JavascriptExceptionOperators::AutoCatchHandlerExists autoCatchHandlerExists(scriptContext);
+            void * addrOfReturnAddr = _AddressOfReturnAddress();
+            Js::JavascriptExceptionOperators::TryHandlerAddrOfReturnAddrStack tryHandlerAddrOfReturnAddrStack(scriptContext, addrOfReturnAddr);
 
 #ifdef ENABLE_SCRIPT_DEBUGGING
             if (this->IsInDebugMode())

lib/Runtime/Language/JavascriptExceptionOperators.cpp

@@ -69,16 +69,16 @@ namespace Js
         m_threadContext->SetIsUserCode(m_previousCatchHandlerToUserCodeStatus);
     }
 
-    JavascriptExceptionOperators::TryCatchFrameAddrStack::TryCatchFrameAddrStack(ScriptContext* scriptContext, void *frameAddr)
+    JavascriptExceptionOperators::TryHandlerAddrOfReturnAddrStack::TryHandlerAddrOfReturnAddrStack(ScriptContext* scriptContext, void *addrOfReturnAddr)
     {
         m_threadContext = scriptContext->GetThreadContext();
-        m_prevTryCatchFrameAddr = m_threadContext->GetTryCatchFrameAddr();
-        scriptContext->GetThreadContext()->SetTryCatchFrameAddr(frameAddr);
+        m_prevTryHandlerAddrOfReturnAddr = m_threadContext->GetTryHandlerAddrOfReturnAddr();
+        scriptContext->GetThreadContext()->SetTryHandlerAddrOfReturnAddr(addrOfReturnAddr);
     }
 
-    JavascriptExceptionOperators::TryCatchFrameAddrStack::~TryCatchFrameAddrStack()
+    JavascriptExceptionOperators::TryHandlerAddrOfReturnAddrStack::~TryHandlerAddrOfReturnAddrStack()
     {
-        m_threadContext->SetTryCatchFrameAddr(m_prevTryCatchFrameAddr);
+        m_threadContext->SetTryHandlerAddrOfReturnAddr(m_prevTryHandlerAddrOfReturnAddr);
     }
 
     JavascriptExceptionOperators::HasBailedOutPtrStack::HasBailedOutPtrStack(ScriptContext* scriptContext, bool *hasBailedOutPtr)
@@ -125,13 +125,14 @@ namespace Js
     {
         void *continuation = nullptr;
         JavascriptExceptionObject *exception = nullptr;
-        void *tryCatchFrameAddr = nullptr;
+        void *tryHandlerAddrOfReturnAddr = nullptr;
 
         Js::JavascriptExceptionOperators::HasBailedOutPtrStack hasBailedOutPtrStack(scriptContext, (bool*)((char*)frame + hasBailedOutOffset));
 
         PROBE_STACK(scriptContext, Constants::MinStackJitEHBailout + spillSize + argsSize);
         {
-            Js::JavascriptExceptionOperators::TryCatchFrameAddrStack tryCatchFrameAddrStack(scriptContext, frame);
+            void * addrOfReturnAddr = (void*)((char*)frame + sizeof(char*));
+            Js::JavascriptExceptionOperators::TryHandlerAddrOfReturnAddrStack tryHandlerAddrOfReturnAddrStack(scriptContext, addrOfReturnAddr);
             try
             {
                 Js::JavascriptExceptionOperators::AutoCatchHandlerExists autoCatchHandlerExists(scriptContext);
@@ -140,8 +141,7 @@ namespace Js
             catch (const Js::JavascriptException& err)
             {
                 exception = err.GetAndClear();
-                tryCatchFrameAddr = scriptContext->GetThreadContext()->GetTryCatchFrameAddr();
-                Assert(frame == tryCatchFrameAddr);
+                tryHandlerAddrOfReturnAddr = scriptContext->GetThreadContext()->GetTryHandlerAddrOfReturnAddr();
             }
         }
         if (exception)
@@ -157,7 +157,7 @@ namespace Js
 #if ENABLE_NATIVE_CODEGEN
             if (exception->GetExceptionContext() && exception->GetExceptionContext()->ThrowingFunction())
             {
-                WalkStackForCleaningUpInlineeInfo(scriptContext, nullptr /* start stackwalk from the current frame */, tryCatchFrameAddr);
+                WalkStackForCleaningUpInlineeInfo(scriptContext, nullptr /* start stackwalk from the current frame */, tryHandlerAddrOfReturnAddr);
             }
 #endif
 
@@ -212,11 +212,11 @@ namespace Js
         if (exception)
         {
 #if ENABLE_NATIVE_CODEGEN
-            if (scriptContext->GetThreadContext()->GetTryCatchFrameAddr() != nullptr)
+            if (scriptContext->GetThreadContext()->GetTryHandlerAddrOfReturnAddr() != nullptr)
             {
                 if (exception->GetExceptionContext() && exception->GetExceptionContext()->ThrowingFunction())
                 {
-                    WalkStackForCleaningUpInlineeInfo(scriptContext, nullptr /* start stackwalk from the current frame */, scriptContext->GetThreadContext()->GetTryCatchFrameAddr());
+                    WalkStackForCleaningUpInlineeInfo(scriptContext, nullptr /* start stackwalk from the current frame */, scriptContext->GetThreadContext()->GetTryHandlerAddrOfReturnAddr());
                 }
             }
             else
@@ -295,13 +295,13 @@ namespace Js
     {
         void *continuation = nullptr;
         JavascriptExceptionObject *exception = nullptr;
-        void * tryCatchFrameAddr = nullptr;
+        void * tryHandlerAddrOfReturnAddr = nullptr;
         Js::JavascriptExceptionOperators::HasBailedOutPtrStack hasBailedOutPtrStack(scriptContext, (bool*)((char*)localsPtr + hasBailedOutOffset));
 
         PROBE_STACK(scriptContext, Constants::MinStackJitEHBailout + argsSize);
         {
-            Js::JavascriptExceptionOperators::TryCatchFrameAddrStack tryCatchFrameAddrStack(scriptContext, framePtr);
-
+            void * addrOfReturnAddr = (void*)((char*)framePtr + sizeof(char*));
+            Js::JavascriptExceptionOperators::TryHandlerAddrOfReturnAddrStack tryHandlerAddrOfReturnAddrStack(scriptContext, addrOfReturnAddr);
             try
             {
                 Js::JavascriptExceptionOperators::AutoCatchHandlerExists autoCatchHandlerExists(scriptContext);
@@ -314,8 +314,7 @@ namespace Js
             catch (const Js::JavascriptException& err)
             {
                 exception = err.GetAndClear();
-                tryCatchFrameAddr = scriptContext->GetThreadContext()->GetTryCatchFrameAddr();
-                Assert(framePtr == tryCatchFrameAddr);
+                tryHandlerAddrOfReturnAddr = scriptContext->GetThreadContext()->GetTryHandlerAddrOfReturnAddr();
             }
         }
 
@@ -332,7 +331,7 @@ namespace Js
 #if ENABLE_NATIVE_CODEGEN
             if (exception->GetExceptionContext() && exception->GetExceptionContext()->ThrowingFunction())
             {
-                WalkStackForCleaningUpInlineeInfo(scriptContext, nullptr /* start stackwalk from the current frame */, tryCatchFrameAddr);
+                WalkStackForCleaningUpInlineeInfo(scriptContext, nullptr /* start stackwalk from the current frame */, tryHandlerAddrOfReturnAddr);
             }
 #endif
             exception = exception->CloneIfStaticExceptionObject(scriptContext);
@@ -387,11 +386,11 @@ namespace Js
         if (exception)
         {
 #if ENABLE_NATIVE_CODEGEN
-            if (scriptContext->GetThreadContext()->GetTryCatchFrameAddr() != nullptr)
+            if (scriptContext->GetThreadContext()->GetTryHandlerAddrOfReturnAddr() != nullptr)
             {
                 if (exception->GetExceptionContext() && exception->GetExceptionContext()->ThrowingFunction())
                 {
-                    WalkStackForCleaningUpInlineeInfo(scriptContext, nullptr /* start stackwalk from the current frame */, scriptContext->GetThreadContext()->GetTryCatchFrameAddr());
+                    WalkStackForCleaningUpInlineeInfo(scriptContext, nullptr /* start stackwalk from the current frame */, scriptContext->GetThreadContext()->GetTryHandlerAddrOfReturnAddr());
                 }
             }
             else
@@ -486,14 +485,14 @@ namespace Js
     {
         void* continuationAddr = NULL;
         Js::JavascriptExceptionObject* pExceptionObject = NULL;
-        void *tryCatchFrameAddr = nullptr;
+        void *tryHandlerAddrOfReturnAddr = nullptr;
 
         Js::JavascriptExceptionOperators::HasBailedOutPtrStack hasBailedOutPtrStack(scriptContext, (bool*)((char*)framePtr + hasBailedOutOffset));
 
         PROBE_STACK(scriptContext, Constants::MinStackJitEHBailout);
         {
-            Js::JavascriptExceptionOperators::TryCatchFrameAddrStack tryCatchFrameAddrStack(scriptContext, framePtr);
-
+            void * addrOfReturnAddr = (void*)((char*)framePtr + sizeof(char*));
+            Js::JavascriptExceptionOperators::TryHandlerAddrOfReturnAddrStack tryHandlerAddrOfReturnAddrStack(scriptContext, addrOfReturnAddr);
             try
             {
                 Js::JavascriptExceptionOperators::AutoCatchHandlerExists autoCatchHandlerExists(scriptContext);
@@ -557,8 +556,7 @@ namespace Js
             catch (const Js::JavascriptException& err)
             {
                 pExceptionObject = err.GetAndClear();
-                tryCatchFrameAddr = scriptContext->GetThreadContext()->GetTryCatchFrameAddr();
-                Assert(framePtr == tryCatchFrameAddr);
+                tryHandlerAddrOfReturnAddr = scriptContext->GetThreadContext()->GetTryHandlerAddrOfReturnAddr();
             }
         }
 
@@ -576,7 +574,7 @@ namespace Js
 #if ENABLE_NATIVE_CODEGEN
             if (pExceptionObject->GetExceptionContext() && pExceptionObject->GetExceptionContext()->ThrowingFunction())
             {
-                WalkStackForCleaningUpInlineeInfo(scriptContext, nullptr /* start stackwalk from the current frame */, tryCatchFrameAddr);
+                WalkStackForCleaningUpInlineeInfo(scriptContext, nullptr /* start stackwalk from the current frame */, tryHandlerAddrOfReturnAddr);
             }
 #endif
             pExceptionObject = pExceptionObject->CloneIfStaticExceptionObject(scriptContext);
@@ -722,11 +720,11 @@ namespace Js
         if (pExceptionObject)
         {
 #if ENABLE_NATIVE_CODEGEN
-            if (scriptContext->GetThreadContext()->GetTryCatchFrameAddr() != nullptr)
+            if (scriptContext->GetThreadContext()->GetTryHandlerAddrOfReturnAddr() != nullptr)
             {
                 if (pExceptionObject->GetExceptionContext() && pExceptionObject->GetExceptionContext()->ThrowingFunction())
                 {
-                    WalkStackForCleaningUpInlineeInfo(scriptContext, nullptr /* start stackwalk from the current frame */, scriptContext->GetThreadContext()->GetTryCatchFrameAddr());
+                    WalkStackForCleaningUpInlineeInfo(scriptContext, nullptr /* start stackwalk from the current frame */, scriptContext->GetThreadContext()->GetTryHandlerAddrOfReturnAddr());
                 }
             }
             else
@@ -1053,14 +1051,14 @@ namespace Js
     }
 #if ENABLE_NATIVE_CODEGEN
     // TODO: Add code address of throwing function on exception context, and use that for returnAddress instead of passing nullptr which starts stackwalk from the top
-    void JavascriptExceptionOperators::WalkStackForCleaningUpInlineeInfo(ScriptContext *scriptContext, PVOID returnAddress, PVOID tryCatchFrameAddr)
+    void JavascriptExceptionOperators::WalkStackForCleaningUpInlineeInfo(ScriptContext *scriptContext, PVOID returnAddress, PVOID tryHandlerAddrOfReturnAddr)
     {
-        Assert(tryCatchFrameAddr != nullptr);
+        Assert(tryHandlerAddrOfReturnAddr != nullptr);
         JavascriptStackWalker walker(scriptContext, /*useEERContext*/ true, returnAddress);
 
         // We have to walk the inlinee frames and clear callinfo count on them on an exception
         // At this point inlinedFrameWalker is closed, so we should build it again by calling InlinedFrameWalker::FromPhysicalFrame
-        walker.WalkAndClearInlineeFrameCallInfoOnException(tryCatchFrameAddr);
+        walker.WalkAndClearInlineeFrameCallInfoOnException(tryHandlerAddrOfReturnAddr);
     }
 #endif
     void

lib/Runtime/Language/JavascriptExceptionOperators.h

@@ -43,15 +43,15 @@ namespace Js
             ~AutoCatchHandlerExists();
         };
 
-        class TryCatchFrameAddrStack
+        class TryHandlerAddrOfReturnAddrStack
         {
           private:
-            void * m_prevTryCatchFrameAddr;
+            void * m_prevTryHandlerAddrOfReturnAddr;
             ThreadContext* m_threadContext;
 
           public:
-            TryCatchFrameAddrStack(ScriptContext* scriptContext, void *frameAddr);
-            ~TryCatchFrameAddrStack();
+            TryHandlerAddrOfReturnAddrStack(ScriptContext* scriptContext, void *addrOfReturnAddr);
+            ~TryHandlerAddrOfReturnAddrStack();
         };
 
         class HasBailedOutPtrStack

lib/Runtime/Language/JavascriptStackWalker.cpp

@@ -632,7 +632,7 @@ namespace Js
         return nullptr;
     }
 #if ENABLE_NATIVE_CODEGEN
-    void JavascriptStackWalker::WalkAndClearInlineeFrameCallInfoOnException(void *tryCatchFrameAddr)
+    void JavascriptStackWalker::WalkAndClearInlineeFrameCallInfoOnException(void *tryHandlerAddrOfReturnAddr)
     {
         // Walk the stack and when we find the first native frame, we clear the inlinee's callinfo for this frame
         // It is sufficient we stop at the first native frame which had the enclosing try-catch
@@ -649,10 +649,10 @@ namespace Js
                         inlinedFrame->callInfo.Clear();
                     }
                 }
-                if (this->currentFrame.GetFrame() == tryCatchFrameAddr)
-                {
-                    break;
-                }
+            }
+            if (this->currentFrame.GetAddressOfReturnAddress() == tryHandlerAddrOfReturnAddr)
+            {
+                break;
             }
         }
     }

lib/Runtime/Language/JavascriptStackWalker.h

@@ -237,7 +237,7 @@ namespace Js
         void ClearCachedInternalFrameInfo();
         void SetCachedInternalFrameInfo(InternalFrameType frameType, JavascriptFunction* function, bool hasInlinedFramesOnStack, bool prevIntFrameIsFromBailout);
         InternalFrameInfo GetCachedInternalFrameInfo() const { return this->lastInternalFrameInfo; }
-        void WalkAndClearInlineeFrameCallInfoOnException(void *tryCatchFrameAddr);
+        void WalkAndClearInlineeFrameCallInfoOnException(void *tryHandlerAddrOfReturnAddr);
 #endif
         bool IsCurrentPhysicalFrameForLoopBody() const;