Change GetItem impls to set *value to undefined on false ret paths

Updated GetItemReference, GetProperty, and GetPropertyReference
implementations to set *value to undefined on false return paths as
well.

Also added a unit test from the CVE repro.

lib/Runtime/Language/JavascriptOperators.cpp

@@ -1596,6 +1596,7 @@ CommonNumber:
                 }
             }
 #endif
+            *value = requestContext->GetLibrary()->GetUndefined();
             return FALSE;
         }
     }
@@ -1636,6 +1637,7 @@ CommonNumber:
             }
             object = JavascriptOperators::GetPrototypeNoTrap(object);
         }
+        *value = requestContext->GetLibrary()->GetUndefined();
         return FALSE;
     }
 
@@ -1870,6 +1872,7 @@ CommonNumber:
                 }
             }
 #endif
+            *value = requestContext->GetLibrary()->GetUndefined();
             return foundProperty;
         }
 
@@ -2898,6 +2901,7 @@ CommonNumber:
             }
             object = JavascriptOperators::GetPrototypeNoTrap(object);
         }
+        *value = requestContext->GetLibrary()->GetUndefined();
         return false;
     }
 
@@ -2916,6 +2920,7 @@ CommonNumber:
             }
             object = JavascriptOperators::GetPrototypeNoTrap(object);
         }
+        *value = requestContext->GetLibrary()->GetUndefined();
         return false;
     }
 
@@ -9243,13 +9248,8 @@ CommonNumber:
             }
         }
 
-        if (superRef == nullptr)
-        {
-            // We didn't find a super reference. Emit a reference error.
-            JavascriptError::ThrowReferenceError(scriptContext, JSERR_BadSuperReference, L"super");
-        }
-
-        return superRef;
+        // We didn't find a super reference. Emit a reference error.
+        JavascriptError::ThrowReferenceError(scriptContext, JSERR_BadSuperReference, L"super");
     }
 
     Var JavascriptOperators::OP_ScopedLdSuper(Var scriptFunction, ScriptContext * scriptContext)

lib/Runtime/Library/ArgumentsObject.cpp

@@ -349,6 +349,7 @@ namespace Js
             return true;
         }
 
+        *value = requestContext->GetLibrary()->GetUndefined();
         return false;
     }
 
@@ -364,6 +365,7 @@ namespace Js
             return true;
         }
 
+        *value = requestContext->GetLibrary()->GetUndefined();
         return false;
     }
 

lib/Runtime/Library/JavascriptProxy.cpp

@@ -496,7 +496,11 @@ namespace Js
         auto getPropertyId = [&]()->PropertyId {return propertyId; };
         PropertyDescriptor result;
         BOOL foundProperty = GetPropertyTrap(originalInstance, &result, fn, getPropertyId, requestContext);
-        if (foundProperty && result.IsFromProxy())
+        if (!foundProperty)
+        {
+            *value = requestContext->GetLibrary()->GetUndefined();
+        }
+        else if (result.IsFromProxy())
         {
             *value = GetValueFromDescriptor(RecyclableObject::FromVar(originalInstance), result, requestContext);
         }
@@ -519,7 +523,11 @@ namespace Js
         };
         PropertyDescriptor result;
         BOOL foundProperty = GetPropertyTrap(originalInstance, &result, fn, getPropertyId, requestContext);
-        if (foundProperty && result.IsFromProxy())
+        if (!foundProperty)
+        {
+            *value = requestContext->GetLibrary()->GetUndefined();
+        }
+        else if (result.IsFromProxy())
         {
             *value = GetValueFromDescriptor(RecyclableObject::FromVar(originalInstance), result, requestContext);
         }
@@ -563,7 +571,11 @@ namespace Js
         auto getPropertyId = [&]() -> PropertyId {return propertyId; };
         PropertyDescriptor result;
         BOOL foundProperty = GetPropertyTrap(originalInstance, &result, fn, getPropertyId, requestContext);
-        if (foundProperty && result.IsFromProxy())
+        if (!foundProperty)
+        {
+            *value = requestContext->GetLibrary()->GetUndefined();
+        }
+        else if (result.IsFromProxy())
         {
             *value = GetValueFromDescriptor(RecyclableObject::FromVar(originalInstance), result, requestContext);
         }
@@ -784,7 +796,11 @@ namespace Js
         };
         PropertyDescriptor result;
         BOOL foundProperty = GetPropertyTrap(originalInstance, &result, fn, getPropertyId, requestContext);
-        if (foundProperty && result.IsFromProxy())
+        if (!foundProperty)
+        {
+            *value = requestContext->GetLibrary()->GetUndefined();
+        }
+        else if (result.IsFromProxy())
         {
             *value = GetValueFromDescriptor(RecyclableObject::FromVar(originalInstance), result, requestContext);
         }
@@ -803,7 +819,11 @@ namespace Js
         };
         PropertyDescriptor result;
         BOOL foundProperty = GetPropertyTrap(originalInstance, &result, fn, getPropertyId, requestContext);
-        if (foundProperty && result.IsFromProxy())
+        if (!foundProperty)
+        {
+            *value = requestContext->GetLibrary()->GetUndefined();
+        }
+        else if (result.IsFromProxy())
         {
             *value = GetValueFromDescriptor(RecyclableObject::FromVar(originalInstance), result, requestContext);
         }

lib/Runtime/Library/JavascriptSIMDFloat64x2.cpp

@@ -65,6 +65,8 @@ namespace Js
         {
             return true;
         }
+
+        *value = requestContext->GetLibrary()->GetUndefined();
         return false;
     }
 
@@ -85,6 +87,7 @@ namespace Js
             return true;
         }
 
+        *value = requestContext->GetLibrary()->GetUndefined();
         return false;
     }
 

lib/Runtime/Library/JavascriptString.cpp

@@ -3505,27 +3505,30 @@ case_2:
 
     BOOL JavascriptString::GetProperty(Var originalInstance, PropertyId propertyId, Var* value, PropertyValueInfo* info, ScriptContext* requestContext)
     {
-        return GetPropertyBuiltIns(propertyId, value);
+        return GetPropertyBuiltIns(propertyId, value, requestContext);
     }
     BOOL JavascriptString::GetProperty(Var originalInstance, JavascriptString* propertyNameString, Var* value, PropertyValueInfo* info, ScriptContext* requestContext)
     {
         PropertyRecord const* propertyRecord;
         this->GetScriptContext()->FindPropertyRecord(propertyNameString, &propertyRecord);
 
-        if (propertyRecord != nullptr && GetPropertyBuiltIns(propertyRecord->GetPropertyId(), value))
+        if (propertyRecord != nullptr && GetPropertyBuiltIns(propertyRecord->GetPropertyId(), value, requestContext))
         {
             return true;
         }
+
+        *value = requestContext->GetLibrary()->GetUndefined();
         return false;
     }
-    bool JavascriptString::GetPropertyBuiltIns(PropertyId propertyId, Var* value)
+    bool JavascriptString::GetPropertyBuiltIns(PropertyId propertyId, Var* value, ScriptContext* requestContext)
     {
         if (propertyId == PropertyIds::length)
         {
-            *value = JavascriptNumber::ToVar(this->GetLength(), this->GetScriptContext());
+            *value = JavascriptNumber::ToVar(this->GetLength(), requestContext);
             return true;
         }
 
+        *value = requestContext->GetLibrary()->GetUndefined();
         return false;
     }
     BOOL JavascriptString::GetPropertyReference(Var originalInstance, PropertyId propertyId, Var* value, PropertyValueInfo* info, ScriptContext* requestContext)

lib/Runtime/Library/JavascriptString.h

@@ -160,7 +160,7 @@ namespace Js
         template <typename T, bool copyBuffer>
         static JavascriptString* NewWithBufferT(const wchar_t * content, charcount_t charLength, ScriptContext * scriptContext);
 
-        bool GetPropertyBuiltIns(PropertyId propertyId, Var* value);
+        bool GetPropertyBuiltIns(PropertyId propertyId, Var* value, ScriptContext* scriptContext);
         static const char stringToIntegerMap[128];
         static const uint8 maxUintStringLengthTable[37];
     protected:

lib/Runtime/Library/JavascriptStringObject.cpp

@@ -250,6 +250,7 @@ namespace Js
             return str->GetItemAt(index, value);
         }
 
+        *value = requestContext->GetLibrary()->GetUndefined();
         return false;
     }
 

lib/Runtime/Library/JavascriptWeakMap.cpp

@@ -27,7 +27,7 @@ namespace Js
     JavascriptWeakMap::WeakMapKeyMap* JavascriptWeakMap::GetWeakMapKeyMapFromKey(DynamicObject* key) const
     {
         Var weakMapKeyData = nullptr;
-        if (!key->GetInternalProperty(key, InternalPropertyIds::WeakMapKeyMap, &weakMapKeyData, nullptr, nullptr))
+        if (!key->GetInternalProperty(key, InternalPropertyIds::WeakMapKeyMap, &weakMapKeyData, nullptr, key->GetScriptContext()))
         {
             return nullptr;
         }
@@ -40,7 +40,7 @@ namespace Js
         // The internal property may exist on an object that has had DynamicObject::ResetObject called on itself.
         // In that case the value stored in the property slot should be null.
         DebugOnly(Var unused = nullptr);
-        Assert(!key->GetInternalProperty(key, InternalPropertyIds::WeakMapKeyMap, &unused, nullptr, nullptr) || unused == nullptr);
+        Assert(!key->GetInternalProperty(key, InternalPropertyIds::WeakMapKeyMap, &unused, nullptr, key->GetScriptContext()) || unused == nullptr);
 
         WeakMapKeyMap* weakMapKeyData = RecyclerNew(GetScriptContext()->GetRecycler(), WeakMapKeyMap, GetScriptContext()->GetRecycler());
         BOOL success = key->SetInternalProperty(InternalPropertyIds::WeakMapKeyMap, weakMapKeyData, PropertyOperation_Force, nullptr);

lib/Runtime/Library/moduleroot.cpp

@@ -415,6 +415,7 @@ namespace Js
         {
             return TRUE;
         }
+        *value = requestContext->GetLibrary()->GetUndefined();
         return FALSE;
     }
 
@@ -442,6 +443,7 @@ namespace Js
         {
             return TRUE;
         }
+        *value = requestContext->GetLibrary()->GetUndefined();
         return FALSE;
     }
 

lib/Runtime/Types/DeferredTypeHandler.h

@@ -258,10 +258,12 @@ namespace Js
     {
         if (DeferredTypeFilter::HasFilter() && !DeferredTypeFilter::HasProperty(propertyId))
         {
+            *value = requestContext->GetLibrary()->GetUndefined();
             return false;
         }
         if (!EnsureObjectReady(instance, DeferredInitializeMode_Default))
         {
+            *value = requestContext->GetLibrary()->GetUndefined();
             return FALSE;
         }
         return GetCurrentTypeHandler(instance)->GetProperty(instance, originalInstance, propertyId, value, info, requestContext);
@@ -273,6 +275,7 @@ namespace Js
     {
         if (!EnsureObjectReady(instance, DeferredInitializeMode_Default))
         {
+            *value = requestContext->GetLibrary()->GetUndefined();
             return FALSE;
         }
         return GetCurrentTypeHandler(instance)->GetProperty(instance, originalInstance, propertyNameString, value, info, requestContext);
@@ -393,6 +396,7 @@ namespace Js
     {
         if (!EnsureObjectReady(instance, DeferredInitializeMode_Default))
         {
+            *value = requestContext->GetLibrary()->GetUndefined();
             return FALSE;
         }
         return GetCurrentTypeHandler(instance)->GetItem(instance, originalInstance, index, value, requestContext);

lib/Runtime/Types/DictionaryTypeHandler.cpp

@@ -547,6 +547,7 @@ namespace Js
             return DictionaryTypeHandlerBase<T>::GetItem(instance, originalInstance, propertyRecord->GetNumericValue(), value, requestContext);
         }
 
+        *value = requestContext->GetLibrary()->GetUndefined();
         return false;
     }
 
@@ -564,6 +565,7 @@ namespace Js
             return GetPropertyFromDescriptor<false>(instance, originalInstance, descriptor, value, info, propertyName, requestContext);
         }
 
+        *value = requestContext->GetLibrary()->GetUndefined();
         return false;
     }
 

lib/Runtime/Types/DynamicObject.cpp

@@ -167,6 +167,7 @@ namespace Js
 
     BOOL DynamicObject::GetObjectArrayItem(Var originalInstance, uint32 index, Var* value, ScriptContext* requestContext)
     {
+        *value = requestContext->GetLibrary()->GetUndefined();
         return HasObjectArray() && GetObjectArrayOrFlagsAsArray()->GetItem(originalInstance, index, value, requestContext);
     }
 
@@ -705,19 +706,19 @@ namespace Js
         //     of being wrapped in CrossSite<>.
 
         Var stackTraceValue = nullptr;
-        if (this->GetInternalProperty(this, InternalPropertyIds::StackTrace, &stackTraceValue, nullptr, nullptr))
+        if (this->GetInternalProperty(this, InternalPropertyIds::StackTrace, &stackTraceValue, nullptr, this->GetScriptContext()))
         {
             this->SetInternalProperty(InternalPropertyIds::StackTrace, nullptr, PropertyOperation_None, nullptr);
         }
 
         Var weakMapKeyMapValue = nullptr;
-        if (this->GetInternalProperty(this, InternalPropertyIds::WeakMapKeyMap, &weakMapKeyMapValue, nullptr, nullptr))
+        if (this->GetInternalProperty(this, InternalPropertyIds::WeakMapKeyMap, &weakMapKeyMapValue, nullptr, this->GetScriptContext()))
         {
             this->SetInternalProperty(InternalPropertyIds::WeakMapKeyMap, nullptr, PropertyOperation_Force, nullptr);
         }
 
         Var mutationBpValue = nullptr;
-        if (this->GetInternalProperty(this, InternalPropertyIds::MutationBp, &mutationBpValue, nullptr, nullptr))
+        if (this->GetInternalProperty(this, InternalPropertyIds::MutationBp, &mutationBpValue, nullptr, this->GetScriptContext()))
         {
             this->SetInternalProperty(InternalPropertyIds::MutationBp, nullptr, PropertyOperation_Force, nullptr);
         }

lib/Runtime/Types/DynamicType.cpp

@@ -140,7 +140,7 @@ namespace Js
     BOOL DynamicObject::GetInternalProperty(Var originalInstance, PropertyId propertyId, Var* value, PropertyValueInfo* info, ScriptContext* requestContext)
     {
         Assert(Js::IsInternalPropertyId(propertyId));
-        return GetTypeHandler()->GetProperty(this, originalInstance, propertyId, value, nullptr, nullptr);
+        return GetTypeHandler()->GetProperty(this, originalInstance, propertyId, value, nullptr, requestContext);
     }
 
     BOOL DynamicObject::GetPropertyReference(Var originalInstance, PropertyId propertyId, Var* value, PropertyValueInfo* info, ScriptContext* requestContext)

lib/Runtime/Types/ES5ArrayTypeHandler.cpp

@@ -674,6 +674,7 @@ namespace Js
         {
             if (descriptor->Attributes & PropertyDeleted)
             {
+                *value = requestContext->GetLibrary()->GetUndefined();
                 return false;
             }
 
@@ -684,11 +685,12 @@ namespace Js
             }
             else
             {
-                *value = instance->GetLibrary()->GetUndefined();
+                *value = requestContext->GetLibrary()->GetUndefined();
             }
             return true;
         }
 
+        *value = requestContext->GetLibrary()->GetUndefined();
         return false;
     }
 

lib/Runtime/Types/MissingPropertyTypeHandler.cpp

@@ -72,11 +72,13 @@ namespace Js
 
     BOOL MissingPropertyTypeHandler::GetProperty(DynamicObject* instance, Var originalInstance, PropertyId propertyId, Var* value, PropertyValueInfo* info, ScriptContext* requestContext)
     {
+        *value = requestContext->GetLibrary()->GetUndefined();
         return false;
     }
 
     BOOL MissingPropertyTypeHandler::GetProperty(DynamicObject* instance, Var originalInstance, JavascriptString* propertyNameString, Var* value, PropertyValueInfo* info, ScriptContext* requestContext)
     {
+        *value = requestContext->GetLibrary()->GetUndefined();
         return false;
     }
 

lib/Runtime/Types/NullTypeHandler.cpp

@@ -107,6 +107,7 @@ namespace Js
             return DynamicTypeHandler::GetItem(instance, originalInstance, indexVal, value, requestContext);
         }
 
+        *value = requestContext->GetLibrary()->GetUndefined();
         return false;
     }
 

lib/Runtime/Types/PathTypeHandler.cpp

@@ -203,6 +203,7 @@ namespace Js
             return PathTypeHandlerBase::GetItem(instance, originalInstance, indexVal, value, requestContext);
         }
 
+        *value = requestContext->GetLibrary()->GetUndefined();
         return false;
     }
 
@@ -223,6 +224,7 @@ namespace Js
             requestContext->FindPropertyRecord(propertyName, propertyNameLength, &propertyRecord);
             if (propertyRecord == nullptr)
             {
+                *value = requestContext->GetLibrary()->GetUndefined();
                 return false;
             }
         }

lib/Runtime/Types/SimpleDictionaryTypeHandler.cpp

@@ -1155,6 +1155,7 @@ namespace Js
             return SimpleDictionaryTypeHandlerBase<TPropertyIndex, TMapKey, IsNotExtensibleSupported>::GetItem(instance, originalInstance, propertyRecord->GetNumericValue(), value, requestContext);
         }
 
+        *value = requestContext->GetLibrary()->GetUndefined();
         return false;
     }
 
@@ -1171,6 +1172,7 @@ namespace Js
             return GetPropertyFromDescriptor<false>(instance, descriptor, value, info);
         }
 
+        *value = requestContext->GetLibrary()->GetUndefined();
         return false;
     }
 

lib/Runtime/Types/SimpleTypeHandler.cpp

@@ -395,6 +395,7 @@ namespace Js
             {
                 if (descriptors[i].Attributes & PropertyDeleted)
                 {
+                    *value = requestContext->GetLibrary()->GetUndefined();
                     return false;
                 }
                 *value = instance->GetSlot(i);
@@ -411,6 +412,7 @@ namespace Js
             return SimpleTypeHandler<size>::GetItem(instance, originalInstance, indexVal, value, scriptContext);
         }
 
+        *value = requestContext->GetLibrary()->GetUndefined();
         return false;
     }
 
@@ -427,6 +429,7 @@ namespace Js
             {
                 if (descriptors[i].Attributes & PropertyDeleted)
                 {
+                    *value = requestContext->GetLibrary()->GetUndefined();
                     return false;
                 }
                 *value = instance->GetSlot(i);
@@ -435,6 +438,7 @@ namespace Js
             }
         }
 
+        *value = requestContext->GetLibrary()->GetUndefined();
         return false;
     }
 

test/es6/proxybug.js

@@ -0,0 +1,12 @@
+//-------------------------------------------------------------------------------------------------------
+// Copyright (C) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
+//-------------------------------------------------------------------------------------------------------
+
+var bug = new Proxy(new Array(1), {has: () => true});
+var a = bug.concat();
+if (a[0] !== undefined || a.length !== 1) {
+    print("failed");
+} else {
+    print("passed");
+}

test/es6/rlexe.xml

@@ -683,10 +683,14 @@
       <files>proxybug3.js</files>
     </default>
   </test>
+  <test>
+    <default>
+      <files>proxybug.js</files>
+    </default>
+  </test>
   <test>
     <default>
       <files>proxyenumbug.js</files>
-      <compile-flags>-args summary -endargs</compile-flags>
     </default>
   </test>
   <test>