diff --git a/dom/base/nsDOMClassInfo.cpp b/dom/base/nsDOMClassInfo.cpp index 3d53da5fda5..5199a9bf036 100644 --- a/dom/base/nsDOMClassInfo.cpp +++ b/dom/base/nsDOMClassInfo.cpp @@ -7197,7 +7197,8 @@ nsWindowSH::NewResolve(nsIXPConnectWrappedNative *wrapper, JSContext *cx, // method on an interface that would let us just call into the // window code directly... - if (!ObjectIsNativeWrapper(cx, obj)) { + if (!ObjectIsNativeWrapper(cx, obj) || + xpc::WrapperFactory::XrayWrapperNotShadowing(obj, id)) { nsCOMPtr dsn(do_QueryInterface(win->GetDocShell())); int32_t count = 0; @@ -9221,7 +9222,8 @@ nsHTMLDocumentSH::NewResolve(nsIXPConnectWrappedNative *wrapper, JSContext *cx, JSAutoRequest ar(cx); - if (!ObjectIsNativeWrapper(cx, obj)) { + if (!ObjectIsNativeWrapper(cx, obj) || + xpc::WrapperFactory::XrayWrapperNotShadowing(obj, id)) { nsCOMPtr result; nsWrapperCache *cache; nsresult rv = ResolveImpl(cx, wrapper, id, getter_AddRefs(result), @@ -9313,23 +9315,20 @@ nsHTMLDocumentSH::GetProperty(nsIXPConnectWrappedNative *wrapper, JSContext *cx, JSObject *obj, jsid id, jsval *vp, bool *_retval) { - // For native wrappers, do not get random names on document - if (!ObjectIsNativeWrapper(cx, obj)) { - nsCOMPtr result; + nsCOMPtr result; - JSAutoRequest ar(cx); + JSAutoRequest ar(cx); - nsWrapperCache *cache; - nsresult rv = ResolveImpl(cx, wrapper, id, getter_AddRefs(result), &cache); - NS_ENSURE_SUCCESS(rv, rv); + nsWrapperCache *cache; + nsresult rv = ResolveImpl(cx, wrapper, id, getter_AddRefs(result), &cache); + NS_ENSURE_SUCCESS(rv, rv); - if (result) { - rv = WrapNative(cx, obj, result, cache, true, vp); - if (NS_SUCCEEDED(rv)) { - rv = NS_SUCCESS_I_DID_SOMETHING; - } - return rv; + if (result) { + rv = WrapNative(cx, obj, result, cache, true, vp); + if (NS_SUCCEEDED(rv)) { + rv = NS_SUCCESS_I_DID_SOMETHING; } + return rv; } return NS_OK; @@ -9371,7 +9370,8 @@ nsHTMLFormElementSH::NewResolve(nsIXPConnectWrappedNative *wrapper, { // For native wrappers, do not resolve random names on form if ((!(JSRESOLVE_ASSIGNING & flags)) && JSID_IS_STRING(id) && - !ObjectIsNativeWrapper(cx, obj)) { + (!ObjectIsNativeWrapper(cx, obj) || + xpc::WrapperFactory::XrayWrapperNotShadowing(obj, id))) { nsCOMPtr form(do_QueryWrappedNative(wrapper, obj)); nsCOMPtr result; nsWrapperCache *cache; @@ -9402,18 +9402,16 @@ nsHTMLFormElementSH::GetProperty(nsIXPConnectWrappedNative *wrapper, if (JSID_IS_STRING(id)) { // For native wrappers, do not get random names on form - if (!ObjectIsNativeWrapper(cx, obj)) { - nsCOMPtr result; - nsWrapperCache *cache; + nsCOMPtr result; + nsWrapperCache *cache; - FindNamedItem(form, id, getter_AddRefs(result), &cache); + FindNamedItem(form, id, getter_AddRefs(result), &cache); - if (result) { - // Wrap result, result can be either an element or a list of - // elements - nsresult rv = WrapNative(cx, obj, result, cache, true, vp); - return NS_FAILED(rv) ? rv : NS_SUCCESS_I_DID_SOMETHING; - } + if (result) { + // Wrap result, result can be either an element or a list of + // elements + nsresult rv = WrapNative(cx, obj, result, cache, true, vp); + return NS_FAILED(rv) ? rv : NS_SUCCESS_I_DID_SOMETHING; } } else { int32_t n = GetArrayIndexFromId(cx, id); diff --git a/dom/base/nsDOMClassInfo.h b/dom/base/nsDOMClassInfo.h index eda41a24104..5db97878487 100644 --- a/dom/base/nsDOMClassInfo.h +++ b/dom/base/nsDOMClassInfo.h @@ -132,6 +132,11 @@ public: * dealing with document.domain, it's possible to end up in a scriptable * helper with a wrapper, even though we should be treating the lookup as a * transparent one. + * + * Note: So ObjectIsNativeWrapper(cx, obj) check usually means "through xray + * wrapper this part is not visible" while combined with + * || xpc::WrapperFactory::XrayWrapperNotShadowing(obj) it means "through + * xray wrapper it is visible only if it does not hide any native property." */ static bool ObjectIsNativeWrapper(JSContext* cx, JSObject* obj); diff --git a/js/xpconnect/tests/chrome/Makefile.in b/js/xpconnect/tests/chrome/Makefile.in index 58ddb32bd89..17eb76441e4 100644 --- a/js/xpconnect/tests/chrome/Makefile.in +++ b/js/xpconnect/tests/chrome/Makefile.in @@ -32,6 +32,7 @@ MOCHITEST_CHROME_FILES = \ test_bug679861.xul \ test_bug706301.xul \ test_bug726949.xul \ + test_bug738244.xul \ test_bug743843.xul \ test_bug760076.xul \ test_bug760109.xul \ diff --git a/js/xpconnect/tests/chrome/test_bug738244.xul b/js/xpconnect/tests/chrome/test_bug738244.xul new file mode 100644 index 00000000000..0835ad75bee --- /dev/null +++ b/js/xpconnect/tests/chrome/test_bug738244.xul @@ -0,0 +1,50 @@ + + + + + + + + + + + + + + + + diff --git a/js/xpconnect/tests/mochitest/Makefile.in b/js/xpconnect/tests/mochitest/Makefile.in index 358c9e45836..74ef1269563 100644 --- a/js/xpconnect/tests/mochitest/Makefile.in +++ b/js/xpconnect/tests/mochitest/Makefile.in @@ -72,6 +72,7 @@ MOCHITEST_FILES = bug500931_helper.html \ file_empty.html \ file_documentdomain.html \ test_lookupMethod.html \ + file_bug738244.html \ $(NULL) MOCHITEST_CHROME_FILES = \ diff --git a/js/xpconnect/tests/mochitest/file_bug738244.html b/js/xpconnect/tests/mochitest/file_bug738244.html new file mode 100644 index 00000000000..a399d9f0e0f --- /dev/null +++ b/js/xpconnect/tests/mochitest/file_bug738244.html @@ -0,0 +1,10 @@ + + +
+ + +
+ + + diff --git a/js/xpconnect/wrappers/WrapperFactory.cpp b/js/xpconnect/wrappers/WrapperFactory.cpp index a4868576737..ff24ea69c5c 100644 --- a/js/xpconnect/wrappers/WrapperFactory.cpp +++ b/js/xpconnect/wrappers/WrapperFactory.cpp @@ -625,6 +625,14 @@ WrapperFactory::WrapForSameCompartmentXray(JSContext *cx, JSObject *obj) return wrapperObj; } + +bool +WrapperFactory::XrayWrapperNotShadowing(JSObject *wrapper, jsid id) +{ + ResolvingId *rid = ResolvingId::getResolvingIdFromWrapper(wrapper); + return rid->isXrayShadowing(id); +} + /* * Calls to JS_TransplantObject* should go through these helpers here so that * waivers get fixed up properly. diff --git a/js/xpconnect/wrappers/WrapperFactory.h b/js/xpconnect/wrappers/WrapperFactory.h index b93c6d829fe..588a2582bf3 100644 --- a/js/xpconnect/wrappers/WrapperFactory.h +++ b/js/xpconnect/wrappers/WrapperFactory.h @@ -93,6 +93,9 @@ class WrapperFactory { // Wrap a same-compartment object for Xray inspection. static JSObject *WrapForSameCompartmentXray(JSContext *cx, JSObject *obj); + + // Returns true if the wrapper is in not shadowing mode for the id. + static bool XrayWrapperNotShadowing(JSObject *wrapper, jsid id); }; extern js::DirectWrapper XrayWaiver; diff --git a/js/xpconnect/wrappers/XrayWrapper.cpp b/js/xpconnect/wrappers/XrayWrapper.cpp index d55f58e0374..4db3d84b764 100644 --- a/js/xpconnect/wrappers/XrayWrapper.cpp +++ b/js/xpconnect/wrappers/XrayWrapper.cpp @@ -279,6 +279,68 @@ createHolder(JSContext *cx, JSObject *wrappedNative, JSObject *parent) using namespace XrayUtils; +ResolvingId::ResolvingId(JSObject *wrapper, jsid id) + : mId(id), + mHolder(getHolderObject(wrapper)), + mPrev(getResolvingId(mHolder)), + mXrayShadowing(false) +{ + js::SetReservedSlot(mHolder, JSSLOT_RESOLVING, js::PrivateValue(this)); +} + +ResolvingId::~ResolvingId() +{ + MOZ_ASSERT(getResolvingId(mHolder) == this, "unbalanced ResolvingIds"); + js::SetReservedSlot(mHolder, JSSLOT_RESOLVING, js::PrivateValue(mPrev)); +} + +bool +ResolvingId::isXrayShadowing(jsid id) +{ + if (!mXrayShadowing) + return false; + + return mId == id; +} + +bool +ResolvingId::isResolving(jsid id) +{ + for (ResolvingId *cur = this; cur; cur = cur->mPrev) { + if (cur->mId == id) + return true; + } + + return false; +} + +ResolvingId * +ResolvingId::getResolvingId(JSObject *holder) +{ + MOZ_ASSERT(strcmp(JS_GetClass(holder)->name, "NativePropertyHolder") == 0); + return (ResolvingId *)js::GetReservedSlot(holder, JSSLOT_RESOLVING).toPrivate(); +} + +JSObject * +ResolvingId::getHolderObject(JSObject *wrapper) +{ + return &js::GetProxyExtra(wrapper, 0).toObject(); +} + +ResolvingId * +ResolvingId::getResolvingIdFromWrapper(JSObject *wrapper) +{ + return getResolvingId(getHolderObject(wrapper)); +} + +class ResolvingIdDummy +{ +public: + ResolvingIdDummy(JSObject *wrapper, jsid id) + { + } +}; + class XPCWrappedNativeXrayTraits { public: @@ -298,30 +360,18 @@ public: } static JSObject* getInnerObject(JSObject *wrapper); - class ResolvingId - { - public: - ResolvingId(JSObject *holder, jsid id); - ~ResolvingId(); - - private: - friend class XPCWrappedNativeXrayTraits; - - jsid mId; - JSObject *mHolder; - ResolvingId *mPrev; - }; static bool isResolving(JSContext *cx, JSObject *holder, jsid id); + static bool resolveDOMCollectionProperty(JSContext *cx, JSObject *wrapper, JSObject *holder, + jsid id, bool set, PropertyDescriptor *desc); + + typedef ResolvingId ResolvingIdImpl; + private: static JSObject* getHolderObject(JSObject *wrapper) { return &js::GetProxyExtra(wrapper, 0).toObject(); } - static ResolvingId* getResolvingId(JSObject *holder) - { - return (ResolvingId *)js::GetReservedSlot(holder, JSSLOT_RESOLVING).toPrivate(); - } }; class ProxyXrayTraits @@ -346,18 +396,13 @@ public: return &js::GetProxyPrivate(wrapper).toObject(); } - class ResolvingId - { - public: - ResolvingId(JSObject *holder, jsid id) - { - } - }; static bool isResolving(JSContext *cx, JSObject *holder, jsid id) { - return false; + return false; } + typedef ResolvingIdDummy ResolvingIdImpl; + private: static JSObject* getHolderObject(JSContext *cx, JSObject *wrapper, bool createHolder) @@ -395,18 +440,13 @@ public: return &js::GetProxyPrivate(wrapper).toObject(); } - class ResolvingId - { - public: - ResolvingId(JSObject *holder, jsid id) - { - } - }; static bool isResolving(JSContext *cx, JSObject *holder, jsid id) { - return false; + return false; } + typedef ResolvingIdDummy ResolvingIdImpl; + private: static JSObject* getHolderObject(JSContext *cx, JSObject *wrapper, bool createHolder) @@ -472,30 +512,11 @@ XPCWrappedNativeXrayTraits::getInnerObject(JSObject *wrapper) return GetWrappedNativeObjectFromHolder(getHolderObject(wrapper)); } -XPCWrappedNativeXrayTraits::ResolvingId::ResolvingId(JSObject *wrapper, jsid id) - : mId(id), - mHolder(getHolderObject(wrapper)), - mPrev(getResolvingId(mHolder)) -{ - js::SetReservedSlot(mHolder, JSSLOT_RESOLVING, PrivateValue(this)); -} - -XPCWrappedNativeXrayTraits::ResolvingId::~ResolvingId() -{ - NS_ASSERTION(getResolvingId(mHolder) == this, "unbalanced ResolvingIds"); - js::SetReservedSlot(mHolder, JSSLOT_RESOLVING, PrivateValue(mPrev)); -} - bool XPCWrappedNativeXrayTraits::isResolving(JSContext *cx, JSObject *holder, jsid id) { - for (ResolvingId *cur = getResolvingId(holder); cur; cur = cur->mPrev) { - if (cur->mId == id) - return true; - } - - return false; + return ResolvingId::getResolvingId(holder)->isResolving(id); } // Some DOM objects have shared properties that don't have an explicit @@ -548,6 +569,71 @@ holder_set(JSContext *cx, JSHandleObject wrapper_, JSHandleId id, JSBool strict, return true; } +class AutoSetWrapperNotShadowing +{ +public: + AutoSetWrapperNotShadowing(JSObject *wrapper MOZ_GUARD_OBJECT_NOTIFIER_PARAM) + { + MOZ_GUARD_OBJECT_NOTIFIER_INIT; + MOZ_ASSERT(wrapper); + mResolvingId = ResolvingId::getResolvingIdFromWrapper(wrapper); + MOZ_ASSERT(mResolvingId); + mResolvingId->mXrayShadowing = true; + } + + ~AutoSetWrapperNotShadowing() + { + mResolvingId->mXrayShadowing = false; + } + +private: + MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER + ResolvingId *mResolvingId; +}; + +// This is called after the resolveNativeProperty could not find any property +// with the given id. At this point we can check for DOM specific collections +// like document["formName"] because we already know that it is not shadowing +// any native property. +bool +XPCWrappedNativeXrayTraits::resolveDOMCollectionProperty(JSContext *cx, JSObject *wrapper, + JSObject *holder, jsid id, bool set, + PropertyDescriptor *desc) +{ + // If we are not currently resolving this id and resolveNative is called + // we don't do anything. (see defineProperty in case of shadowing is forbidden). + ResolvingId *rid = ResolvingId::getResolvingId(holder); + if (!rid || rid->mId != id) + return true; + + XPCWrappedNative *wn = GetWrappedNativeFromHolder(holder); + if (!NATIVE_HAS_FLAG(wn, WantNewResolve)) + return true; + + // Setting the current ResolvingId in non-shadowing mode. So for this id + // Xray won't ignore DOM specific collection properties temporarily. + AutoSetWrapperNotShadowing asw(wrapper); + + bool retval = true; + JSObject *pobj = NULL; + unsigned flags = (set ? JSRESOLVE_ASSIGNING : 0) | JSRESOLVE_QUALIFIED; + nsresult rv = wn->GetScriptableInfo()->GetCallback()->NewResolve(wn, cx, wrapper, id, + flags, &pobj, &retval); + if (NS_FAILED(rv)) { + if (retval) + XPCThrower::Throw(rv, cx); + return false; + } + + if (pobj && !JS_GetPropertyDescriptorById(cx, holder, id, + JSRESOLVE_QUALIFIED, desc)) + { + return false; + } + + return true; +} + bool XPCWrappedNativeXrayTraits::resolveNativeProperty(JSContext *cx, JSObject *wrapper, JSObject *holder, jsid id, bool set, @@ -562,11 +648,13 @@ XPCWrappedNativeXrayTraits::resolveNativeProperty(JSContext *cx, JSObject *wrapp // This will do verification and the method lookup for us. XPCCallContext ccx(JS_CALLER, cx, wnObject, nullptr, id); - // There are no native numeric properties, so we can shortcut here. We will not - // find the property. + // There are no native numeric properties, so we can shortcut here. We will + // not find the property. However we want to support non shadowing dom + // specific collection properties like window.frames, so we still have to + // check for those. if (!JSID_IS_STRING(id)) { /* Not found */ - return true; + return resolveDOMCollectionProperty(cx, wrapper, holder, id, set, desc); } XPCNativeInterface *iface; @@ -576,7 +664,7 @@ XPCWrappedNativeXrayTraits::resolveNativeProperty(JSContext *cx, JSObject *wrapp !(iface = ccx.GetInterface()) || !(member = ccx.GetMember())) { /* Not found */ - return true; + return resolveDOMCollectionProperty(cx, wrapper, holder, id, set, desc); } desc->obj = holder; @@ -1235,7 +1323,7 @@ XrayWrapper::getPropertyDescriptor(JSContext *cx, JSObject *wrappe if (!this->enter(cx, wrapper, id, action, &status)) return status; - typename Traits::ResolvingId resolving(wrapper, id); + typename Traits::ResolvingIdImpl resolving(wrapper, id); // Redirect access straight to the wrapper if we should be transparent. if (XrayUtils::IsTransparent(cx, wrapper)) { @@ -1334,7 +1422,7 @@ XrayWrapper::getOwnPropertyDescriptor(JSContext *cx, JSObject *wra if (!this->enter(cx, wrapper, id, action, &status)) return status; - typename Traits::ResolvingId resolving(wrapper, id); + typename Traits::ResolvingIdImpl resolving(wrapper, id); // NB: Nothing we do here acts on the wrapped native itself, so we don't // enter our policy. diff --git a/js/xpconnect/wrappers/XrayWrapper.h b/js/xpconnect/wrappers/XrayWrapper.h index 5ebb4f435ce..0ce938cf356 100644 --- a/js/xpconnect/wrappers/XrayWrapper.h +++ b/js/xpconnect/wrappers/XrayWrapper.h @@ -7,6 +7,7 @@ #include "jsapi.h" #include "jswrapper.h" +#include "mozilla/GuardObjects.h" // Xray wrappers re-resolve the original native properties on the native // object and always directly access to those properties. @@ -100,4 +101,30 @@ public: }; extern SandboxProxyHandler sandboxProxyHandler; + +class AutoSetWrapperNotShadowing; +class XPCWrappedNativeXrayTraits; + +class ResolvingId { +public: + ResolvingId(JSObject *wrapper, jsid id); + ~ResolvingId(); + + bool isXrayShadowing(jsid id); + bool isResolving(jsid id); + static ResolvingId* getResolvingId(JSObject *holder); + static JSObject* getHolderObject(JSObject *wrapper); + static ResolvingId *getResolvingIdFromWrapper(JSObject *wrapper); + +private: + friend class AutoSetWrapperNotShadowing; + friend class XPCWrappedNativeXrayTraits; + + jsid mId; + JSObject *mHolder; + ResolvingId *mPrev; + bool mXrayShadowing; +}; + } +