[CVE-2018-0773] An uaf bug in the latest version of Edge,lead to rce - Qihoo 360

Dynamic import keyword should have been disabled. The bug exploits our use of JavascriptError::SetErrorMessageProperties with the underlying buffer from a JavascriptString object which might get collected leading to use after free of that buffer.

Fix here disables dynamic import feature and fixes the exploit since some of that code is shared with other components.

lib/Common/ConfigFlagsList.h

@@ -611,6 +611,7 @@ PHASE(All)
 #define DEFAULT_CONFIG_TraceAsyncDebugCalls     (false)
 #define DEFAULT_CONFIG_ForcePostLowerGlobOptInstrString (false)
 #define DEFAULT_CONFIG_EnumerateSpecialPropertiesInDebugger (true)
+#define DEFAULT_CONFIG_ESDynamicImport         (false)
 #endif
 
 #define DEFAULT_CONFIG_MaxJITFunctionBytecodeByteLength (4800000)
@@ -1029,6 +1030,11 @@ FLAGPR           (Boolean, ES6, ES7TrailingComma       , "Enable ES7 trailing co
 FLAGPR           (Boolean, ES6, ES6IsConcatSpreadable  , "Enable ES6 isConcatSpreadable Symbol"                     , DEFAULT_CONFIG_ES6IsConcatSpreadable)
 FLAGPR           (Boolean, ES6, ES6Math                , "Enable ES6 Math extensions"                               , DEFAULT_CONFIG_ES6Math)
 
+#ifndef COMPILE_DISABLE_ESDynamicImport
+#define COMPILE_DISABLE_ESDynamicImport 0
+#endif
+FLAGPR_REGOVR_EXP(Boolean, ES6, ESDynamicImport        , "Enable dynamic import"                                    , DEFAULT_CONFIG_ESDynamicImport)
+
 FLAGPR           (Boolean, ES6, ES6Module              , "Enable ES6 Modules"                                       , DEFAULT_CONFIG_ES6Module)
 FLAGPR           (Boolean, ES6, ES6Object              , "Enable ES6 Object extensions"                             , DEFAULT_CONFIG_ES6Object)
 FLAGPR           (Boolean, ES6, ES6Number              , "Enable ES6 Number extensions"                             , DEFAULT_CONFIG_ES6Number)

lib/Parser/Parse.cpp

@@ -2657,6 +2657,11 @@ ParseNodePtr Parser::ParseImport()
     // import()
     if (m_token.tk == tkLParen)
     {
+        if (!m_scriptContext->GetConfig()->IsESDynamicImportEnabled())
+        {
+            Error(ERRsyntax);
+        }
+
         ParseNodePtr pnode = ParseImportCall<buildAST>();
         BOOL fCanAssign;
         IdentToken token;
@@ -3528,7 +3533,7 @@ LFunction :
         break;
 
     case tkIMPORT:
-        if (m_scriptContext->GetConfig()->IsES6ModuleEnabled())
+        if (m_scriptContext->GetConfig()->IsES6ModuleEnabled() && m_scriptContext->GetConfig()->IsESDynamicImportEnabled())
         {
             m_pscan->Scan();
             ChkCurTokNoScan(tkLParen, ERRnoLparen);

lib/Runtime/Base/ThreadConfigFlagsList.h

@@ -47,6 +47,7 @@ FLAG_RELEASE(IsES7AsyncAndAwaitEnabled, ES7AsyncAwait)
 FLAG_RELEASE(IsArrayBufferTransferEnabled, ArrayBufferTransfer)
 FLAG_RELEASE(IsESObjectGetOwnPropertyDescriptorsEnabled, ESObjectGetOwnPropertyDescriptors)
 FLAG_RELEASE(IsESSharedArrayBufferEnabled, ESSharedArrayBuffer)
+FLAG_RELEASE(IsESDynamicImportEnabled, ESDynamicImport)
 #ifdef ENABLE_PROJECTION
 FLAG(AreWinRTDelegatesInterfaces, WinRTDelegateInterfaces)
 FLAG_RELEASE(IsWinRTAdaptiveAppsEnabled, WinRTAdaptiveApps)

lib/Runtime/Language/JavascriptOperators.cpp

@@ -9653,8 +9653,14 @@ CommonNumber:
 
         if (FAILED(hr))
         {
+            // We cannot just use the buffer in the specifier string - need to make a copy here.
+            size_t length = wcslen(moduleName);
+            char16* allocatedString = RecyclerNewArrayLeaf(scriptContext->GetRecycler(), char16, length + 1);
+            wmemcpy_s(allocatedString, length + 1, moduleName, length);
+            allocatedString[length] = _u('\0');
+
             Js::JavascriptError *error = scriptContext->GetLibrary()->CreateURIError();
-            JavascriptError::SetErrorMessageProperties(error, hr, moduleName, scriptContext);
+            JavascriptError::SetErrorMessageProperties(error, hr, allocatedString, scriptContext);
             return SourceTextModuleRecord::ResolveOrRejectDynamicImportPromise(false, error, scriptContext);
         }
 

lib/Runtime/Language/SourceTextModuleRecord.cpp

@@ -113,8 +113,16 @@ namespace Js
             Assert(sourceLength == 0);
             OUTPUT_TRACE_DEBUGONLY(Js::ModulePhase, _u("Failed to load: %s\n"), this->GetSpecifierSz());
             hr = E_FAIL;
+
+            // We cannot just use the buffer in the specifier string - need to make a copy here.
+            const char16* moduleName = this->GetSpecifierSz();
+            size_t length = wcslen(moduleName);
+            char16* allocatedString = RecyclerNewArrayLeaf(scriptContext->GetRecycler(), char16, length + 1);
+            wmemcpy_s(allocatedString, length + 1, moduleName, length);
+            allocatedString[length] = _u('\0');
+
             JavascriptError *pError = scriptContext->GetLibrary()->CreateURIError();
-            JavascriptError::SetErrorMessageProperties(pError, hr, this->GetSpecifierSz(), scriptContext);
+            JavascriptError::SetErrorMessageProperties(pError, hr, allocatedString, scriptContext);
             *exceptionVar = pError;
         }
         else
@@ -288,8 +296,15 @@ namespace Js
 
             if (FAILED(hr))
             {
+                // We cannot just use the buffer in the specifier string - need to make a copy here.
+                const char16* moduleName = this->GetSpecifierSz();
+                size_t length = wcslen(moduleName);
+                char16* allocatedString = RecyclerNewArrayLeaf(scriptContext->GetRecycler(), char16, length + 1);
+                wmemcpy_s(allocatedString, length + 1, moduleName, length);
+                allocatedString[length] = _u('\0');
+
                 Js::JavascriptError * error = scriptContext->GetLibrary()->CreateURIError();
-                JavascriptError::SetErrorMessageProperties(error, hr, this->GetSpecifierSz(), scriptContext);
+                JavascriptError::SetErrorMessageProperties(error, hr, allocatedString, scriptContext);
                 return SourceTextModuleRecord::ResolveOrRejectDynamicImportPromise(false, error, scriptContext, this);
             }
         }

test/es6/rlexe.xml

@@ -1350,14 +1350,14 @@
 <test>
     <default>
         <files>dynamic-module-functionality.js</files>
-        <compile-flags>-ES6Module -args summary -endargs</compile-flags>
+        <compile-flags>-ES6Module -ESDynamicImport -args summary -endargs</compile-flags>
         <tags>exclude_sanitize_address</tags>
     </default>
 </test>
 <test>
     <default>
         <files>dynamic-module-import-specifier.js</files>
-        <compile-flags>-MuteHostErrorMsg -ES6Module -args summary -endargs</compile-flags>
+        <compile-flags>-MuteHostErrorMsg -ES6Module -ESDynamicImport -args summary -endargs</compile-flags>
         <tags>exclude_sanitize_address</tags>
     </default>
 </test>
@@ -1393,7 +1393,7 @@
 <test>
     <default>
         <files>bug_OS12095746.js</files>
-        <compile-flags>-MuteHostErrorMsg -IgnoreScriptErrorCode -TraceHostCallback -ES6Module</compile-flags>
+        <compile-flags>-MuteHostErrorMsg -IgnoreScriptErrorCode -TraceHostCallback -ES6Module -ESDynamicImport</compile-flags>
         <tags>exclude_dynapogo,exclude_sanitize_address,bugfix,exclude_drt</tags>
         <baseline>bug_OS12095746.baseline</baseline>
     </default>
@@ -1442,7 +1442,7 @@
 <test>
   <default>
     <files>bug_issue_3076.js</files>
-    <compile-flags>-force:deferparse</compile-flags>
+    <compile-flags>-force:deferparse -ESDynamicImport</compile-flags>
     <tags>BugFix,exclude_sanitize_address</tags>
   </default>
 </test>
@@ -1456,6 +1456,7 @@
 <test>
   <default>
     <files>bug_issue_3257.js</files>
+    <compile-flags>-ESDynamicImport</compile-flags>
     <baseline>bug_issue_3257.baseline</baseline>
     <tags>BugFix,exclude_sanitize_address</tags>
   </default>