[CVE-2018-8298] Edge - Chakra: Bugs in InitializeNumberFormat and InitializeDateTimeFormat - Google, Inc.

Author: jackhorton

lib/Runtime/ByteCode/ByteCodeCacheReleaseFileVersion.h

@@ -4,6 +4,6 @@
 //-------------------------------------------------------------------------------------------------------
 // NOTE: If there is a merge conflict the correct fix is to make a new GUID.
 
-// {2E95A003-1442-404F-98D5-D5C973B8A719}
+// {18949169-1B93-4123-B34A-F42F1C1EAF9A}
 const GUID byteCodeCacheReleaseFileVersion =
-{ 0x2E95A003, 0x1442, 0x404F, { 0x98, 0xD5, 0xD5, 0xC9, 0x73, 0xB8, 0xA7, 0x19 } };
+{ 0x18949169, 0x1B93, 0x4123, { 0xB3, 0x4A, 0xF4, 0x2F, 0x1C, 0x1E, 0xAF, 0x9A } };

lib/Runtime/InternalPropertyList.h

@@ -19,7 +19,11 @@ INTERNALPROPERTY(FrozenType)                      // Used to store shared frozen
 INTERNALPROPERTY(StackTrace)                      // Stack trace object for Error.stack generation
 INTERNALPROPERTY(StackTraceCache)                 // Cache of Error.stack string
 INTERNALPROPERTY(WeakMapKeyMap)                   // WeakMap data stored on WeakMap key objects
-INTERNALPROPERTY(HiddenObject)                    // Used to store hidden data for JS library code (Intl as an example will use this)
+INTERNALPROPERTY(HiddenObject)                    // Used to store internal slot data for JS library code (Intl as an example will use this)
+INTERNALPROPERTY(CachedUCollator)                 // Used to store cached UCollator objects for Intl.Collator
+INTERNALPROPERTY(CachedUNumberFormat)             // Used to store cached UNumberFormat objects for Intl.NumberFormat and Intl.PluralRules
+INTERNALPROPERTY(CachedUDateFormat)               // Used to store cached UDateFormat objects for Intl.DateTimeFormat
+INTERNALPROPERTY(CachedUPluralRules)              // Used to store cached UPluralRules objects for Intl.PluralRules
 INTERNALPROPERTY(RevocableProxy)                  // Internal slot for [[RevokableProxy]] for revocable proxy in ES6
 INTERNALPROPERTY(MutationBp)                      // Used to store strong reference to the mutation breakpoint object
 #undef INTERNALPROPERTY

lib/Runtime/Library/InJavascript/Intl.js.bc.32b.h

@@ -743,7 +743,7 @@ namespace Js
             Js::CallInfo callInfo(Js::CallFlags_Value, _countof(args));
 
             Js::Arguments arguments(callInfo, args);
-            scriptContext->GetThreadContext()->ExecuteImplicitCall(function, Js::ImplicitCall_Accessor, [=]()->Js::Var 
+            scriptContext->GetThreadContext()->ExecuteImplicitCall(function, Js::ImplicitCall_Accessor, [=]()->Js::Var
             {
                 return JavascriptFunction::CallRootFunctionInScript(function, arguments);
             });
@@ -1536,7 +1536,7 @@ DEFINE_ISXLOCALEAVAILABLE(PR, uloc)
         }
 
         state->SetInternalProperty(
-            InternalPropertyIds::HiddenObject,
+            InternalPropertyIds::CachedUNumberFormat,
             fmt,
             PropertyOperationFlags::PropertyOperation_None,
             nullptr
@@ -1817,14 +1817,13 @@ DEFINE_ISXLOCALEAVAILABLE(PR, uloc)
         }
 
         // Below, we lazy-initialize the backing UCollator on the first call to localeCompare
-        // On subsequent calls, the UCollator will be cached in state.hiddenObject
-        // TODO(jahorto): Make these property IDs sane, so that hiddenObject doesn't have different meanings in different contexts
-        Var hiddenObject = nullptr;
+        // On subsequent calls, the UCollator will be cached in state.CachedUCollator
+        Var cachedUCollator = nullptr;
         FinalizableUCollator *coll = nullptr;
         UErrorCode status = U_ZERO_ERROR;
-        if (state->GetInternalProperty(state, Js::InternalPropertyIds::HiddenObject, &hiddenObject, nullptr, scriptContext))
+        if (state->GetInternalProperty(state, InternalPropertyIds::CachedUCollator, &cachedUCollator, nullptr, scriptContext))
         {
-            coll = reinterpret_cast<FinalizableUCollator *>(hiddenObject);
+            coll = reinterpret_cast<FinalizableUCollator *>(cachedUCollator);
             INTL_TRACE("Using previously cached UCollator (0x%x)", coll);
         }
         else
@@ -1898,7 +1897,7 @@ DEFINE_ISXLOCALEAVAILABLE(PR, uloc)
 
             // cache coll for later use (so that the condition that brought us here returns true for future calls)
             state->SetInternalProperty(
-                InternalPropertyIds::HiddenObject,
+                InternalPropertyIds::CachedUCollator,
                 coll,
                 PropertyOperationFlags::PropertyOperation_None,
                 nullptr
@@ -2360,8 +2359,8 @@ DEFINE_ISXLOCALEAVAILABLE(PR, uloc)
         DynamicObject *state = DynamicObject::UnsafeFromVar(args[2]);
         bool toParts = JavascriptBoolean::UnsafeFromVar(args[3])->GetValue();
         bool forNumberPrototypeToLocaleString = JavascriptBoolean::UnsafeFromVar(args[4])->GetValue();
-        Var cachedFormatter = nullptr; // cached by EntryIntl_CacheNumberFormat
-        AssertOrFailFast(state->GetInternalProperty(state, Js::InternalPropertyIds::HiddenObject, &cachedFormatter, NULL, scriptContext));
+        Var cachedUNumberFormat = nullptr; // cached by EntryIntl_CacheNumberFormat
+        AssertOrFailFast(state->GetInternalProperty(state, InternalPropertyIds::CachedUNumberFormat, &cachedUNumberFormat, NULL, scriptContext));
 
         if (forNumberPrototypeToLocaleString)
         {
@@ -2379,7 +2378,7 @@ DEFINE_ISXLOCALEAVAILABLE(PR, uloc)
             INTL_TRACE("Calling NumberFormat.prototype.format(%f)", num);
         }
 
-        auto fmt = static_cast<FinalizableUNumberFormat *>(cachedFormatter);
+        auto fmt = static_cast<FinalizableUNumberFormat *>(cachedUNumberFormat);
         char16 *formatted = nullptr;
         int formattedLen = 0;
 
@@ -2620,14 +2619,13 @@ DEFINE_ISXLOCALEAVAILABLE(PR, uloc)
         }
 
         // Below, we lazy-initialize the backing UDateFormat on the first call to format{ToParts}
-        // On subsequent calls, the UDateFormat will be cached in state.hiddenObject
-        // TODO(jahorto): Make these property IDs sane, so that hiddenObject doesn't have different meanings in different contexts
-        Var hiddenObject = nullptr;
+        // On subsequent calls, the UDateFormat will be cached in state.CachedUDateFormat
+        Var cachedUDateFormat = nullptr;
         FinalizableUDateFormat *dtf = nullptr;
         UErrorCode status = U_ZERO_ERROR;
-        if (state->GetInternalProperty(state, Js::InternalPropertyIds::HiddenObject, &hiddenObject, nullptr, scriptContext))
+        if (state->GetInternalProperty(state, InternalPropertyIds::CachedUDateFormat, &cachedUDateFormat, nullptr, scriptContext))
         {
-            dtf = reinterpret_cast<FinalizableUDateFormat *>(hiddenObject);
+            dtf = reinterpret_cast<FinalizableUDateFormat *>(cachedUDateFormat);
             INTL_TRACE("Using previously cached UDateFormat (0x%x)", dtf);
         }
         else
@@ -2668,7 +2666,7 @@ DEFINE_ISXLOCALEAVAILABLE(PR, uloc)
 
             // cache dtf for later use (so that the condition that brought us here returns true for future calls)
             state->SetInternalProperty(
-                InternalPropertyIds::HiddenObject,
+                InternalPropertyIds::CachedUDateFormat,
                 dtf,
                 PropertyOperationFlags::PropertyOperation_None,
                 nullptr
@@ -2949,11 +2947,11 @@ DEFINE_ISXLOCALEAVAILABLE(PR, uloc)
 #ifdef INTL_ICU
     static FinalizableUPluralRules *GetOrCreatePluralRulesCache(DynamicObject *stateObject, ScriptContext *scriptContext)
     {
-        Var hiddenObject = nullptr;
+        Var cachedUPluralRules = nullptr;
         FinalizableUPluralRules *pr = nullptr;
-        if (stateObject->GetInternalProperty(stateObject, InternalPropertyIds::HiddenObject, &hiddenObject, nullptr, scriptContext))
+        if (stateObject->GetInternalProperty(stateObject, InternalPropertyIds::CachedUPluralRules, &cachedUPluralRules, nullptr, scriptContext))
         {
-            pr = reinterpret_cast<FinalizableUPluralRules *>(hiddenObject);
+            pr = reinterpret_cast<FinalizableUPluralRules *>(cachedUPluralRules);
             INTL_TRACE("Using previously cached UPluralRules (0x%x)", pr);
         }
         else
@@ -2981,7 +2979,7 @@ DEFINE_ISXLOCALEAVAILABLE(PR, uloc)
 
             INTL_TRACE("Caching UPluralRules object (0x%x) with langtag %s and type %s", langtag->GetSz(), type->GetSz());
 
-            stateObject->SetInternalProperty(InternalPropertyIds::HiddenObject, pr, PropertyOperationFlags::PropertyOperation_None, nullptr);
+            stateObject->SetInternalProperty(InternalPropertyIds::CachedUPluralRules, pr, PropertyOperationFlags::PropertyOperation_None, nullptr);
         }
 
         return pr;