Repository navigation
vm: access to Symbols on global context does not work across sandbox boundary #884
Description
Activity
Might be more global vs. global proxy stuff (#855), or might be because @isaacs's good ol' hack uses the V8 GetOwnPropertyNames API which probably doesn't give back symbols.
I think it's a bit of both.
v8::Object::GetOwnPropertyNames()indeed doesn't return symbols. That could be scripted around if it weren't for #864 because:v8::Object::GetOwnPropertyNamesis subtly incompatible withObject.getOwnPropertyNames(), see https://code.google.com/p/v8/issues/detail?id=3861- There is no
v8::Object::GetOwnPropertySymbols()counterpart toObject.getOwnPropertySymbols(), see https://code.google.com/p/v8/issues/detail?id=3901 - Thanks to Change from 0.10 => 1.2;
enumerableofthis.escapeinvm.runInNewContextwas false, now true #864, it's not possible to tell built-in properties apart from user-defined ones because everything is enumerable.
Combined, it makes it pretty much impossible to make it work in either C++ or JS. Fixing #864 isn't easy either because you can't look up a property's attributes without going through interceptors (creating infinite recursion) like you can for a property's value.
On the upside, adding a
v8::Object::GetRealNamedPropertyAttributes()method turned out pretty straightforward. When I have some time, I'll try to get it landed upstream.diff --git a/deps/v8/src/api.cc b/deps/v8/src/api.cc index 88d3c88..e16a594 100644 --- a/deps/v8/src/api.cc +++ b/deps/v8/src/api.cc @@ -3774,6 +3774,23 @@ Local<Value> v8::Object::GetRealNamedProperty(Handle<String> key) { } +PropertyAttribute v8::Object::GetRealNamedPropertyAttributes( + Handle<String> key) { + i::Isolate* isolate = Utils::OpenHandle(this)->GetIsolate(); + ON_BAILOUT(isolate, "v8::Object::GetRealNamedPropertyAttributes()", + return static_cast<PropertyAttribute>(NONE)); + ENTER_V8(isolate); + i::Handle<i::JSObject> self_obj = Utils::OpenHandle(this); + i::Handle<i::String> key_obj = Utils::OpenHandle(*key); + i::LookupIterator it(self_obj, key_obj, + i::LookupIterator::PROTOTYPE_CHAIN_SKIP_INTERCEPTOR); + Maybe<PropertyAttributes> result = self_obj->GetPropertyAttributes(&it); + DCHECK(result.has_value); + if (result.value == ABSENT) return static_cast<PropertyAttribute>(NONE); + return static_cast<PropertyAttribute>(result.value); +} + + // Turns on access checks by copying the map and setting the check flag. // Because the object gets a new map, existing inline cache caching // the old map of this object will fail.
One thing that might be helpful in fixing this is using ObjectTemplate::SetHandler instead of SetNamedPropertyHandler. The latter calls the former with PropertyHandlerFlags::kOnlyInterceptStrings which sounds like exactly the opposite of what we want. Going to try it soon...
Seems like this is a change from how v8-master to how it is handled in the version currently in node. Seems like
SetHandlertakes a bool in the currently pulled version, indicating whether it takes into account symbols.Yeah I am working in the next branch on this.
This is harder than it seems because to use v8::Name you need to buy in to the MaybeLocal revolution.
Fixed by 9002cc2.
This is more a guess than anything, but apparently accessing
Symbols in a vm context is not forwarded to the original object handle, resulting in different values depending on which side of the sandbox boundary the access happens.Reduced test case: