Chakra Assert : sym->HasScopeSlot()

Ian Halliday · agarwal-sandeep · commit ff9067ebe9e1 · 2016-06-17T10:18:07.000-07:00
CVE-2016-3202 A let/const bug exposes a bad array indexing operation during bytecode emit. A symbol can fail to get a location due to this bug but then the location is used to offset into an array assuming it is always valid. Fix the array offset issue by performing a bounds check and throwing FatalInternalError if an out of bounds access would occur.
diff --git a/lib/Runtime/ByteCode/ByteCodeEmitter.cpp b/lib/Runtime/ByteCode/ByteCodeEmitter.cpp
@@ -1799,37 +1799,47 @@ void ByteCodeGenerator::InitScopeSlotArray(FuncInfo * funcInfo)
             propertyIdsForScopeSlotArray[i] = Js::Constants::NoProperty;
         }
 #endif
+        auto setPropertyIdForScopeSlotArray =
+            [scopeSlotCount, propertyIdsForScopeSlotArray]
+            (Js::PropertyId slot, Js::PropertyId propId)
+        {
+            if (slot < 0 || (uint)slot >= scopeSlotCount)
+            {
+                Js::Throw::FatalInternalError();
+            }
+            propertyIdsForScopeSlotArray[slot] = propId;
+        };
 
-        auto setPropIdsForScopeSlotArray = [funcInfo, propertyIdsForScopeSlotArray](Symbol *const sym)
+        auto setPropIdsForScopeSlotArray = [funcInfo, setPropertyIdForScopeSlotArray](Symbol *const sym)
         {
             if (sym->NeedsSlotAlloc(funcInfo))
             {
                 // All properties should get correct propertyId here.
                 Assert(sym->HasScopeSlot()); // We can't allocate scope slot now. Any symbol needing scope slot must have allocated it before this point.
-                propertyIdsForScopeSlotArray[sym->GetScopeSlot()] = sym->EnsurePosition(funcInfo);
+                setPropertyIdForScopeSlotArray(sym->GetScopeSlot(), sym->EnsurePosition(funcInfo));
             }
         };
 
         funcInfo->GetBodyScope()->ForEachSymbol(setPropIdsForScopeSlotArray);
 
         if (funcInfo->thisScopeSlot != Js::Constants::NoRegister)
         {
-            propertyIdsForScopeSlotArray[funcInfo->thisScopeSlot] = Js::PropertyIds::_lexicalThisSlotSymbol;
+            setPropertyIdForScopeSlotArray(funcInfo->thisScopeSlot, Js::PropertyIds::_lexicalThisSlotSymbol);
         }
 
         if (funcInfo->newTargetScopeSlot != Js::Constants::NoRegister)
         {
-            propertyIdsForScopeSlotArray[funcInfo->newTargetScopeSlot] = Js::PropertyIds::_lexicalNewTargetSymbol;
+            setPropertyIdForScopeSlotArray(funcInfo->newTargetScopeSlot, Js::PropertyIds::_lexicalNewTargetSymbol);
         }
 
         if (funcInfo->superScopeSlot != Js::Constants::NoRegister)
         {
-            propertyIdsForScopeSlotArray[funcInfo->superScopeSlot] = Js::PropertyIds::_superReferenceSymbol;
+            setPropertyIdForScopeSlotArray(funcInfo->superScopeSlot, Js::PropertyIds::_superReferenceSymbol);
         }
 
         if (funcInfo->superCtorScopeSlot != Js::Constants::NoRegister)
         {
-            propertyIdsForScopeSlotArray[funcInfo->superCtorScopeSlot] = Js::PropertyIds::_superCtorReferenceSymbol;
+            setPropertyIdForScopeSlotArray(funcInfo->superCtorScopeSlot, Js::PropertyIds::_superCtorReferenceSymbol);
         }
 
 #if DEBUG

Original file line number	Diff line number	Diff line change
`@@ -1799,37 +1799,47 @@ void ByteCodeGenerator::InitScopeSlotArray(FuncInfo * funcInfo)`
`1799`	`1799`	`propertyIdsForScopeSlotArray[i] = Js::Constants::NoProperty;`
`1800`	`1800`	`}`
`1801`	`1801`	`#endif`
	`1802`	`+ auto setPropertyIdForScopeSlotArray =`
	`1803`	`+ [scopeSlotCount, propertyIdsForScopeSlotArray]`
	`1804`	`+ (Js::PropertyId slot, Js::PropertyId propId)`
	`1805`	`+ {`
	`1806`	`+ if (slot < 0 \|\| (uint)slot >= scopeSlotCount)`
	`1807`	`+ {`
	`1808`	`+ Js::Throw::FatalInternalError();`
	`1809`	`+ }`
	`1810`	`+ propertyIdsForScopeSlotArray[slot] = propId;`
	`1811`	`+ };`
`1802`	`1812`
`1803`		`- auto setPropIdsForScopeSlotArray = [funcInfo, propertyIdsForScopeSlotArray](Symbol *const sym)`
	`1813`	`+ auto setPropIdsForScopeSlotArray = [funcInfo, setPropertyIdForScopeSlotArray](Symbol *const sym)`
`1804`	`1814`	`{`
`1805`	`1815`	`if (sym->NeedsSlotAlloc(funcInfo))`
`1806`	`1816`	`{`
`1807`	`1817`	`// All properties should get correct propertyId here.`
`1808`	`1818`	`Assert(sym->HasScopeSlot()); // We can't allocate scope slot now. Any symbol needing scope slot must have allocated it before this point.`
`1809`		`- propertyIdsForScopeSlotArray[sym->GetScopeSlot()] = sym->EnsurePosition(funcInfo);`
	`1819`	`+ setPropertyIdForScopeSlotArray(sym->GetScopeSlot(), sym->EnsurePosition(funcInfo));`
`1810`	`1820`	`}`
`1811`	`1821`	`};`
`1812`	`1822`
`1813`	`1823`	`funcInfo->GetBodyScope()->ForEachSymbol(setPropIdsForScopeSlotArray);`
`1814`	`1824`
`1815`	`1825`	`if (funcInfo->thisScopeSlot != Js::Constants::NoRegister)`
`1816`	`1826`	`{`
`1817`		`- propertyIdsForScopeSlotArray[funcInfo->thisScopeSlot] = Js::PropertyIds::_lexicalThisSlotSymbol;`
	`1827`	`+ setPropertyIdForScopeSlotArray(funcInfo->thisScopeSlot, Js::PropertyIds::_lexicalThisSlotSymbol);`
`1818`	`1828`	`}`
`1819`	`1829`
`1820`	`1830`	`if (funcInfo->newTargetScopeSlot != Js::Constants::NoRegister)`
`1821`	`1831`	`{`
`1822`		`- propertyIdsForScopeSlotArray[funcInfo->newTargetScopeSlot] = Js::PropertyIds::_lexicalNewTargetSymbol;`
	`1832`	`+ setPropertyIdForScopeSlotArray(funcInfo->newTargetScopeSlot, Js::PropertyIds::_lexicalNewTargetSymbol);`
`1823`	`1833`	`}`
`1824`	`1834`
`1825`	`1835`	`if (funcInfo->superScopeSlot != Js::Constants::NoRegister)`
`1826`	`1836`	`{`
`1827`		`- propertyIdsForScopeSlotArray[funcInfo->superScopeSlot] = Js::PropertyIds::_superReferenceSymbol;`
	`1837`	`+ setPropertyIdForScopeSlotArray(funcInfo->superScopeSlot, Js::PropertyIds::_superReferenceSymbol);`
`1828`	`1838`	`}`
`1829`	`1839`
`1830`	`1840`	`if (funcInfo->superCtorScopeSlot != Js::Constants::NoRegister)`
`1831`	`1841`	`{`
`1832`		`- propertyIdsForScopeSlotArray[funcInfo->superCtorScopeSlot] = Js::PropertyIds::_superCtorReferenceSymbol;`
	`1842`	`+ setPropertyIdForScopeSlotArray(funcInfo->superCtorScopeSlot, Js::PropertyIds::_superCtorReferenceSymbol);`
`1833`	`1843`	`}`
`1834`	`1844`
`1835`	`1845`	`#if DEBUG`