Skip to content

Commit

Permalink
[1.8>1.9] [MERGE #4431 @tcare] Ensure WithScopeObject is unwrapped fo…
Browse files Browse the repository at this point in the history 
…r scoped __proto__ stores

Merge pull request #4431 from tcare:withproto

Fixes OS: 14291082

Setting __proto__ inside an eval inside a with takes us down a weird path. Usually we would have unwrapped the WithScopeObject automatically by using SetProperty or similar, but __proto__ is special cased (along with some other things like HostDispatch) and will directly fetch the setter function and call it, bypassing the WithScopeObject:: methods (although still keeping the @@unscopables check.)

Fix is to unwrap the object after the @@unscopables object check has finished when getting the __proto__ setter. I tried several other paths but they end up going through the WithScopeObject shims.
  • Loading branch information
@tcare
tcare committed Jan 31, 2018
2 parents 28cc7c2 + ba715a0 commit 9fe64ab
Show file tree
Hide file tree
Showing 5 changed files with 205 additions and 167 deletions.
252 changes: 122 additions & 130 deletions lib/Runtime/Language/JavascriptOperators.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2486,132 +2486,164 @@ namespace Js
return SetProperty_Internal<false>(instance, instance, true, propertyId, newValue, info, requestContext, propertyOperationFlags);
}

template <bool unscopables>
BOOL JavascriptOperators::SetProperty_Internal(Var receiver, RecyclableObject* object, const bool isRoot, PropertyId propertyId, Var newValue, PropertyValueInfo * info, ScriptContext* requestContext, PropertyOperationFlags propertyOperationFlags)
{
if (receiver)
// Returns true if a result was written.
bool JavascriptOperators::SetAccessorOrNonWritableProperty(
Var receiver,
RecyclableObject* object,
PropertyId propertyId,
Var newValue,
PropertyValueInfo * info,
ScriptContext* requestContext,
PropertyOperationFlags propertyOperationFlags,
bool isRoot,
bool allowUndecInConsoleScope,
BOOL *result)
{
*result = FALSE;
Var setterValueOrProxy = nullptr;
DescriptorFlags flags = None;
if ((isRoot && JavascriptOperators::CheckPrototypesForAccessorOrNonWritableRootProperty(object, propertyId, &setterValueOrProxy, &flags, info, requestContext)) ||
(!isRoot && JavascriptOperators::CheckPrototypesForAccessorOrNonWritableProperty(object, propertyId, &setterValueOrProxy, &flags, info, requestContext)))
{
Assert(!TaggedNumber::Is(receiver));
Var setterValueOrProxy = nullptr;
DescriptorFlags flags = None;
if ((isRoot && JavascriptOperators::CheckPrototypesForAccessorOrNonWritableRootProperty(object, propertyId, &setterValueOrProxy, &flags, info, requestContext)) ||
(!isRoot && JavascriptOperators::CheckPrototypesForAccessorOrNonWritableProperty(object, propertyId, &setterValueOrProxy, &flags, info, requestContext)))
if ((flags & Accessor) == Accessor)
{
if ((flags & Accessor) == Accessor)
if (JavascriptError::ThrowIfStrictModeUndefinedSetter(propertyOperationFlags, setterValueOrProxy, requestContext) ||
JavascriptError::ThrowIfNotExtensibleUndefinedSetter(propertyOperationFlags, setterValueOrProxy, requestContext))
{
*result = TRUE;
return true;
}
if (setterValueOrProxy)
{
if (JavascriptError::ThrowIfStrictModeUndefinedSetter(propertyOperationFlags, setterValueOrProxy, requestContext) ||
JavascriptError::ThrowIfNotExtensibleUndefinedSetter(propertyOperationFlags, setterValueOrProxy, requestContext))
RecyclableObject* func = RecyclableObject::FromVar(setterValueOrProxy);
Assert(!info || info->GetFlags() == InlineCacheSetterFlag || info->GetPropertyIndex() == Constants::NoSlot);

if (WithScopeObject::Is(receiver))
{
return TRUE;
receiver = (RecyclableObject::FromVar(receiver))->GetThisObjectOrUnWrap();
}
if (setterValueOrProxy)
else
{
RecyclableObject* func = RecyclableObject::FromVar(setterValueOrProxy);
Assert(!info || info->GetFlags() == InlineCacheSetterFlag || info->GetPropertyIndex() == Constants::NoSlot);

if (WithScopeObject::Is(receiver))
{
receiver = (RecyclableObject::FromVar(receiver))->GetThisObjectOrUnWrap();
}
else
{
CacheOperators::CachePropertyWrite(RecyclableObject::FromVar(receiver), isRoot, object->GetType(), propertyId, info, requestContext);
}
CacheOperators::CachePropertyWrite(RecyclableObject::FromVar(receiver), isRoot, object->GetType(), propertyId, info, requestContext);
}
#ifdef ENABLE_MUTATION_BREAKPOINT
if (MutationBreakpoint::IsFeatureEnabled(requestContext))
{
MutationBreakpoint::HandleSetProperty(requestContext, object, propertyId, newValue);
}
#endif
JavascriptOperators::CallSetter(func, receiver, newValue, requestContext);
if (MutationBreakpoint::IsFeatureEnabled(requestContext))
{
MutationBreakpoint::HandleSetProperty(requestContext, object, propertyId, newValue);
}
return TRUE;
#endif
JavascriptOperators::CallSetter(func, receiver, newValue, requestContext);
}
else if ((flags & Proxy) == Proxy)
{
Assert(JavascriptProxy::Is(setterValueOrProxy));
JavascriptProxy* proxy = JavascriptProxy::FromVar(setterValueOrProxy);
// We can't cache the property at this time. both target and handler can be changed outside of the proxy, so the inline cache needs to be
// invalidate when target, handler, or handler prototype has changed. We don't have a way to achieve this yet.
PropertyValueInfo::SetNoCache(info, proxy);
PropertyValueInfo::DisablePrototypeCache(info, proxy); // We can't cache prototype property either
*result = TRUE;
return true;
}
else if ((flags & Proxy) == Proxy)
{
Assert(JavascriptProxy::Is(setterValueOrProxy));
JavascriptProxy* proxy = JavascriptProxy::FromVar(setterValueOrProxy);
// We can't cache the property at this time. both target and handler can be changed outside of the proxy, so the inline cache needs to be
// invalidate when target, handler, or handler prototype has changed. We don't have a way to achieve this yet.
PropertyValueInfo::SetNoCache(info, proxy);
PropertyValueInfo::DisablePrototypeCache(info, proxy); // We can't cache prototype property either

return proxy->SetPropertyTrap(receiver, JavascriptProxy::SetPropertyTrapKind::SetPropertyKind, propertyId, newValue, requestContext);
}
else
*result = proxy->SetPropertyTrap(receiver, JavascriptProxy::SetPropertyTrapKind::SetPropertyKind, propertyId, newValue, requestContext);
return true;
}
else
{
Assert((flags & Data) == Data && (flags & Writable) == None);
if (!allowUndecInConsoleScope)
{
Assert((flags & Data) == Data && (flags & Writable) == None);
if (flags & Const)
{
JavascriptError::ThrowTypeError(requestContext, ERRAssignmentToConst);
}

JavascriptError::ThrowCantAssign(propertyOperationFlags, requestContext, propertyId);
JavascriptError::ThrowCantAssignIfStrictMode(propertyOperationFlags, requestContext);
return FALSE;

*result = FALSE;
return true;
}
}
else if (!JavascriptOperators::IsObject(receiver))
{
JavascriptError::ThrowCantAssignIfStrictMode(propertyOperationFlags, requestContext);
return FALSE;
}
}
return false;
}

template <bool unscopables>
BOOL JavascriptOperators::SetProperty_Internal(Var receiver, RecyclableObject* object, const bool isRoot, PropertyId propertyId, Var newValue, PropertyValueInfo * info, ScriptContext* requestContext, PropertyOperationFlags propertyOperationFlags)
{
if (receiver == nullptr)
{
return FALSE;
}

Assert(!TaggedNumber::Is(receiver));
BOOL setAccessorResult = FALSE;
if (SetAccessorOrNonWritableProperty(receiver, object, propertyId, newValue, info, requestContext, propertyOperationFlags, isRoot, false, &setAccessorResult))
{
return setAccessorResult;
}
else if (!JavascriptOperators::IsObject(receiver))
{
JavascriptError::ThrowCantAssignIfStrictMode(propertyOperationFlags, requestContext);
return FALSE;
}

#ifdef ENABLE_MUTATION_BREAKPOINT
// Break on mutation if needed
bool doNotUpdateCacheForMbp = MutationBreakpoint::IsFeatureEnabled(requestContext) ?
MutationBreakpoint::HandleSetProperty(requestContext, object, propertyId, newValue) : false;
// Break on mutation if needed
bool doNotUpdateCacheForMbp = MutationBreakpoint::IsFeatureEnabled(requestContext) ?
MutationBreakpoint::HandleSetProperty(requestContext, object, propertyId, newValue) : false;
#endif

// Get the original type before setting the property
Type *typeWithoutProperty = object->GetType();
BOOL didSetProperty = false;
if (isRoot)
{
AssertMsg(JavascriptOperators::GetTypeId(receiver) == TypeIds_GlobalObject
|| JavascriptOperators::GetTypeId(receiver) == TypeIds_ModuleRoot,
"Root must be a global object!");
// Get the original type before setting the property
Type *typeWithoutProperty = object->GetType();
BOOL didSetProperty = false;
if (isRoot)
{
AssertMsg(JavascriptOperators::GetTypeId(receiver) == TypeIds_GlobalObject
|| JavascriptOperators::GetTypeId(receiver) == TypeIds_ModuleRoot,
"Root must be a global object!");

RootObjectBase* rootObject = static_cast<RootObjectBase*>(receiver);
didSetProperty = rootObject->SetRootProperty(propertyId, newValue, propertyOperationFlags, info);
}
else
RootObjectBase* rootObject = static_cast<RootObjectBase*>(receiver);
didSetProperty = rootObject->SetRootProperty(propertyId, newValue, propertyOperationFlags, info);
}
else
{
RecyclableObject* instanceObject = RecyclableObject::FromVar(receiver);
while (!JavascriptOperators::IsNull(instanceObject))
{
RecyclableObject* instanceObject = RecyclableObject::FromVar(receiver);
while (!JavascriptOperators::IsNull(instanceObject))
if (unscopables && JavascriptOperators::IsPropertyUnscopable(instanceObject, propertyId))
{
if (unscopables && JavascriptOperators::IsPropertyUnscopable(instanceObject, propertyId))
break;
}
else
{
didSetProperty = instanceObject->SetProperty(propertyId, newValue, propertyOperationFlags, info);
if (didSetProperty || !unscopables)
{
break;
}
else
{
didSetProperty = instanceObject->SetProperty(propertyId, newValue, propertyOperationFlags, info);
if (didSetProperty || !unscopables)
{
break;
}
}
instanceObject = JavascriptOperators::GetPrototypeNoTrap(instanceObject);
}
instanceObject = JavascriptOperators::GetPrototypeNoTrap(instanceObject);
}
}

if (didSetProperty)
{
bool updateCache = true;
if (didSetProperty)
{
bool updateCache = true;
#ifdef ENABLE_MUTATION_BREAKPOINT
updateCache = updateCache && !doNotUpdateCacheForMbp;
updateCache = updateCache && !doNotUpdateCacheForMbp;
#endif

if (updateCache)
if (updateCache)
{
if (!JavascriptProxy::Is(receiver))
{
if (!JavascriptProxy::Is(receiver))
{
CacheOperators::CachePropertyWrite(RecyclableObject::FromVar(receiver), isRoot, typeWithoutProperty, propertyId, info, requestContext);
}
CacheOperators::CachePropertyWrite(RecyclableObject::FromVar(receiver), isRoot, typeWithoutProperty, propertyId, info, requestContext);
}
return TRUE;
}
return TRUE;
}

return FALSE;
Expand Down Expand Up @@ -2939,50 +2971,10 @@ namespace Js
// is true, this must be a normal property.
// TODO: merge OP_HasProperty and GetSetter in one pass if there is perf problem. In fastDOM we have quite
// a lot of setters so separating the two might be actually faster.
Var setterValueOrProxy = nullptr;
DescriptorFlags flags = None;
if (JavascriptOperators::CheckPrototypesForAccessorOrNonWritableProperty(object, propertyId, &setterValueOrProxy, &flags, &info, scriptContext))
BOOL setAccessorResult = FALSE;
if (SetAccessorOrNonWritableProperty(object, object, propertyId, newValue, &info, scriptContext, propertyOperationFlags, false, allowUndecInConsoleScope, &setAccessorResult))
{
if ((flags & Accessor) == Accessor)
{
if (setterValueOrProxy)
{
JavascriptFunction* func = (JavascriptFunction*)setterValueOrProxy;
Assert(info.GetFlags() == InlineCacheSetterFlag || info.GetPropertyIndex() == Constants::NoSlot);
CacheOperators::CachePropertyWrite(object, false, type, propertyId, &info, scriptContext);
JavascriptOperators::CallSetter(func, object, newValue, scriptContext);
}

Assert(!isLexicalThisSlotSymbol);
return;
}
else if ((flags & Proxy) == Proxy)
{
Assert(JavascriptProxy::Is(setterValueOrProxy));
JavascriptProxy* proxy = JavascriptProxy::FromVar(setterValueOrProxy);
auto fn = [&](RecyclableObject* target) -> BOOL {
return JavascriptOperators::SetProperty(object, target, propertyId, newValue, scriptContext, propertyOperationFlags);
};
// We can't cache the property at this time. both target and handler can be changed outside of the proxy, so the inline cache needs to be
// invalidate when target, handler, or handler prototype has changed. We don't have a way to achieve this yet.
PropertyValueInfo::SetNoCache(&info, proxy);
PropertyValueInfo::DisablePrototypeCache(&info, proxy); // We can't cache prototype property either
proxy->SetPropertyTrap(object, JavascriptProxy::SetPropertyTrapKind::SetPropertyKind, propertyId, newValue, scriptContext);
}
else
{
Assert((flags & Data) == Data && (flags & Writable) == None);
if (!allowUndecInConsoleScope)
{
if (flags & Const)
{
JavascriptError::ThrowTypeError(scriptContext, ERRAssignmentToConst);
}

Assert(!isLexicalThisSlotSymbol);
return;
}
}
return;
}
else if (!JavascriptOperators::IsObject(object))
{
Expand Down
12 changes: 12 additions & 0 deletions lib/Runtime/Language/JavascriptOperators.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -674,6 +674,18 @@ namespace Js
static BOOL PropertyReferenceWalk_Impl(Var instance, RecyclableObject** propertyObject, PropertyId propertyId, Var* value, PropertyValueInfo* info, ScriptContext* requestContext);
static Var TypeofFld_Internal(Var instance, const bool isRoot, PropertyId propertyId, ScriptContext* scriptContext);

static bool SetAccessorOrNonWritableProperty(
Var receiver,
RecyclableObject* object,
PropertyId propertyId,
Var newValue,
PropertyValueInfo * info,
ScriptContext* requestContext,
PropertyOperationFlags propertyOperationFlags,
bool isRoot,
bool allowUndecInConsoleScope,
BOOL *result);

template <bool unscopables>
static BOOL SetProperty_Internal(Var instance, RecyclableObject* object, const bool isRoot, PropertyId propertyId, Var newValue, PropertyValueInfo * info, ScriptContext* requestContext, PropertyOperationFlags flags);

Expand Down
1 change: 1 addition & 0 deletions lib/Runtime/Library/RuntimeLibraryPch.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
#include "Library/JavascriptWeakMap.h"
#include "Library/JavascriptWeakSet.h"

#include "Types/WithScopeObject.h"
#include "Types/PropertyIndexRanges.h"
#include "Types/DictionaryPropertyDescriptor.h"
#include "Types/DictionaryTypeHandler.h"
Expand Down
Loading

0 comments on commit 9fe64ab

Please sign in to comment.