From d9fc29464fd7d7580165e71473c2d5b76ea4ce8b Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Sat, 2 Feb 2019 03:24:45 +0000 Subject: [PATCH] Bug 1521907 part 5. Start using CheckedUnwrapStatic/Dynamic in XPConnect. r=peterv I am not a huge fan of the UnwrapReflectorToISupports setup here. Maybe we should introduce two differently-named methods that make it somewhat clear what the limitations of not taking a JSContext are? I couldn't think of sane naming... Differential Revision: https://phabricator.services.mozilla.com/D17885 --HG-- extra : moz-landing-system : lando --- dom/base/StructuredCloneHolder.cpp | 8 +++-- dom/bindings/BindingUtils.cpp | 9 ++++-- dom/xbl/nsXBLProtoImplField.cpp | 5 +++- js/ipc/WrapperOwner.cpp | 4 ++- js/xpconnect/src/ExportHelpers.cpp | 19 +++++++----- js/xpconnect/src/Sandbox.cpp | 30 ++++++++++++++----- js/xpconnect/src/XPCCallContext.cpp | 3 +- js/xpconnect/src/XPCComponents.cpp | 22 ++++++++++---- js/xpconnect/src/XPCConvert.cpp | 12 ++++---- js/xpconnect/src/XPCJSContext.cpp | 3 +- js/xpconnect/src/XPCJSID.cpp | 20 +++++++++---- js/xpconnect/src/XPCJSWeakReference.cpp | 2 +- js/xpconnect/src/XPCVariant.cpp | 5 ++-- js/xpconnect/src/XPCWrappedNativeJSOps.cpp | 2 +- js/xpconnect/src/nsXPConnect.cpp | 22 ++++++++++---- js/xpconnect/src/xpcpublic.h | 25 ++++++++++++++-- js/xpconnect/wrappers/XrayWrapper.cpp | 8 +++-- .../components/telemetry/core/Stopwatch.cpp | 3 +- 18 files changed, 146 insertions(+), 56 deletions(-) diff --git a/dom/base/StructuredCloneHolder.cpp b/dom/base/StructuredCloneHolder.cpp index 04018b638ebf..f8581ca8cd5c 100644 --- a/dom/base/StructuredCloneHolder.cpp +++ b/dom/base/StructuredCloneHolder.cpp @@ -468,8 +468,9 @@ void StructuredCloneHolder::ReadFromBuffer(nsISupports* aParent, JSContext* aCx, } } - if (NS_IsMainThread() && xpc::IsReflector(obj)) { - nsCOMPtr base = xpc::UnwrapReflectorToISupports(obj); + if (NS_IsMainThread() && xpc::IsReflector(obj, aCx)) { + // We only care about principals, so ReflectorToISupportsStatic is fine. + nsCOMPtr base = xpc::ReflectorToISupportsStatic(obj); nsCOMPtr principal = do_QueryInterface(base); if (principal) { auto nsjsprincipals = nsJSPrincipals::get(principal); @@ -1042,7 +1043,8 @@ bool StructuredCloneHolder::CustomWriteHandler(JSContext* aCx, } { - nsCOMPtr base = xpc::UnwrapReflectorToISupports(aObj); + // We only care about streams, so ReflectorToISupportsStatic is fine. + nsCOMPtr base = xpc::ReflectorToISupportsStatic(aObj); nsCOMPtr inputStream = do_QueryInterface(base); if (inputStream) { return WriteInputStream(aWriter, inputStream, this); diff --git a/dom/bindings/BindingUtils.cpp b/dom/bindings/BindingUtils.cpp index f1b867546539..87a449c8e25f 100644 --- a/dom/bindings/BindingUtils.cpp +++ b/dom/bindings/BindingUtils.cpp @@ -2303,7 +2303,10 @@ nsISupports* GlobalObject::GetAsSupports() const { // IsWrapper bit above and the UnwrapDOMObjectToISupports in the case when // we're not actually an XPCWrappedNative, but this should be a rare-ish case // anyway. - nsCOMPtr supp = xpc::UnwrapReflectorToISupports(mGlobalJSObject); + // + // It's OK to use ReflectorToISupportsStatic, because we know we don't have a + // cross-compartment wrapper. + nsCOMPtr supp = xpc::ReflectorToISupportsStatic(mGlobalJSObject); if (supp) { // See documentation for mGlobalJSObject for why this assignment is OK. mGlobalObject = supp; @@ -3266,7 +3269,9 @@ nsresult UnwrapArgImpl(JSContext* cx, JS::Handle src, return NS_ERROR_NOT_AVAILABLE; } - nsCOMPtr iface = xpc::UnwrapReflectorToISupports(src); + // The JSContext represents the "who is unwrapping" realm, so we want to use + // it for ReflectorToISupportsDynamic here. + nsCOMPtr iface = xpc::ReflectorToISupportsDynamic(src, cx); if (iface) { if (NS_FAILED(iface->QueryInterface(iid, ppArg))) { return NS_ERROR_XPC_BAD_CONVERT_JS; diff --git a/dom/xbl/nsXBLProtoImplField.cpp b/dom/xbl/nsXBLProtoImplField.cpp index 9be3c27df3b0..3fdec0fadfe1 100644 --- a/dom/xbl/nsXBLProtoImplField.cpp +++ b/dom/xbl/nsXBLProtoImplField.cpp @@ -141,7 +141,10 @@ static bool InstallXBLField(JSContext* cx, JS::Handle callee, // But there are some cases where we must accept |thisObj| but not install a // property on it, or otherwise touch it. Hence this split of |this|-vetting // duties. - nsCOMPtr native = xpc::UnwrapReflectorToISupports(thisObj); + // + // OK to use ReflectorToISupportsStatic, because we only care about nodes + // here. + nsCOMPtr native = xpc::ReflectorToISupportsStatic(thisObj); if (!native) { // Looks like whatever |thisObj| is it's not our nsIContent. It might well // be the proto our binding installed, however, where the private is the diff --git a/js/ipc/WrapperOwner.cpp b/js/ipc/WrapperOwner.cpp index a5294bcadbd3..0a056224f8b0 100644 --- a/js/ipc/WrapperOwner.cpp +++ b/js/ipc/WrapperOwner.cpp @@ -1034,7 +1034,9 @@ bool WrapperOwner::ok(JSContext* cx, const ReturnStatus& status, // process from this function. It's sent with the CPOW to the remote process, // where it can be fetched with Components.utils.getCrossProcessWrapperTag. static nsCString GetRemoteObjectTag(JS::Handle obj) { - if (nsCOMPtr supports = xpc::UnwrapReflectorToISupports(obj)) { + // OK to use ReflectorToISupportsStatic, because we only care about docshells + // and documents here. + if (nsCOMPtr supports = xpc::ReflectorToISupportsStatic(obj)) { nsCOMPtr treeItem(do_QueryInterface(supports)); if (treeItem) { return NS_LITERAL_CSTRING("ContentDocShellTreeItem"); diff --git a/js/xpconnect/src/ExportHelpers.cpp b/js/xpconnect/src/ExportHelpers.cpp index d3d34c377e7d..b35386809422 100644 --- a/js/xpconnect/src/ExportHelpers.cpp +++ b/js/xpconnect/src/ExportHelpers.cpp @@ -24,8 +24,8 @@ using namespace JS; namespace xpc { -bool IsReflector(JSObject* obj) { - obj = js::CheckedUnwrap(obj, /* stopAtWindowProxy = */ false); +bool IsReflector(JSObject* obj, JSContext* cx) { + obj = js::CheckedUnwrapDynamic(obj, cx, /* stopAtWindowProxy = */ false); if (!obj) { return false; } @@ -70,7 +70,8 @@ class MOZ_STACK_CLASS StackScopedCloneData : public StructuredCloneHolderBase { RootedObject reflector(aCx, mReflectors[idx]); MOZ_ASSERT(reflector, "No object pointer?"); - MOZ_ASSERT(IsReflector(reflector), "Object pointer must be a reflector!"); + MOZ_ASSERT(IsReflector(reflector, aCx), + "Object pointer must be a reflector!"); if (!JS_WrapObject(aCx, &reflector)) { return nullptr; @@ -146,7 +147,8 @@ class MOZ_STACK_CLASS StackScopedCloneData : public StructuredCloneHolderBase { } } - if ((mOptions->wrapReflectors && IsReflector(aObj)) || IsFileList(aObj)) { + if ((mOptions->wrapReflectors && IsReflector(aObj, aCx)) || + IsFileList(aObj)) { if (!mReflectors.append(aObj)) { return false; } @@ -394,8 +396,10 @@ bool ExportFunction(JSContext* cx, HandleValue vfunction, HandleValue vscope, // * We must subsume the scope we are exporting to. // * We must subsume the function being exported, because the function // forwarder manually circumvents security wrapper CALL restrictions. - targetScope = js::CheckedUnwrap(targetScope); - funObj = js::CheckedUnwrap(funObj); + targetScope = js::CheckedUnwrapDynamic(targetScope, cx); + // For the function we can just CheckedUnwrapStatic, because if it's + // not callable we're going to fail out anyway. + funObj = js::CheckedUnwrapStatic(funObj); if (!targetScope || !funObj) { JS_ReportErrorASCII(cx, "Permission denied to export function into scope"); return false; @@ -480,7 +484,8 @@ bool CreateObjectIn(JSContext* cx, HandleValue vobj, return false; } - RootedObject scope(cx, js::CheckedUnwrap(&vobj.toObject())); + // cx represents the caller Realm. + RootedObject scope(cx, js::CheckedUnwrapDynamic(&vobj.toObject(), cx)); if (!scope) { JS_ReportErrorASCII( cx, "Permission denied to create object in the target scope"); diff --git a/js/xpconnect/src/Sandbox.cpp b/js/xpconnect/src/Sandbox.cpp index 647fdb609924..02892a65d828 100644 --- a/js/xpconnect/src/Sandbox.cpp +++ b/js/xpconnect/src/Sandbox.cpp @@ -345,8 +345,14 @@ static bool SandboxIsProxy(JSContext* cx, unsigned argc, Value* vp) { } RootedObject obj(cx, &args[0].toObject()); - obj = js::CheckedUnwrap(obj); - NS_ENSURE_TRUE(obj, false); + // CheckedUnwrapStatic is OK here, since we only care about whether + // it's a scripted proxy and the things CheckedUnwrapStatic fails on + // are not. + obj = js::CheckedUnwrapStatic(obj); + if (!obj) { + args.rval().setBoolean(false); + return true; + } args.rval().setBoolean(js::IsScriptedProxy(obj)); return true; @@ -664,9 +670,10 @@ bool WrapAccessorFunction(JSContext* cx, Op& op, PropertyDescriptor* desc, } static bool IsMaybeWrappedDOMConstructor(JSObject* obj) { - // We really care about the underlying object here, which might be wrapped - // in cross-compartment wrappers. - obj = js::CheckedUnwrap(obj); + // We really care about the underlying object here, which might be wrapped in + // cross-compartment wrappers. CheckedUnwrapStatic is fine, since we just + // care whether it's a DOM constructor. + obj = js::CheckedUnwrapStatic(obj); if (!obj) { return false; } @@ -1135,7 +1142,11 @@ nsresult xpc::CreateSandboxObject(JSContext* cx, MutableHandleValue vp, bool useSandboxProxy = !!WindowOrNull(js::UncheckedUnwrap(options.proto, false)); if (!useSandboxProxy) { - JSObject* unwrappedProto = js::CheckedUnwrap(options.proto, false); + // We just wrapped options.proto into the compartment of whatever Realm + // is on the cx, so use that same realm for the CheckedUnwrapDynamic + // call. + JSObject* unwrappedProto = + js::CheckedUnwrapDynamic(options.proto, cx, false); if (!unwrappedProto) { JS_ReportErrorASCII(cx, "Sandbox must subsume sandboxPrototype"); return NS_ERROR_INVALID_ARG; @@ -1264,7 +1275,8 @@ static bool GetPrincipalOrSOP(JSContext* cx, HandleObject from, MOZ_ASSERT(out); *out = nullptr; - nsCOMPtr native = xpc::UnwrapReflectorToISupports(from); + // We might have a Window here, so need ReflectorToISupportsDynamic + nsCOMPtr native = ReflectorToISupportsDynamic(from, cx); if (nsCOMPtr sop = do_QueryInterface(native)) { sop.forget(out); @@ -1801,7 +1813,9 @@ nsresult xpc::EvalInSandbox(JSContext* cx, HandleObject sandboxArg, rval.set(UndefinedValue()); bool waiveXray = xpc::WrapperFactory::HasWaiveXrayFlag(sandboxArg); - RootedObject sandbox(cx, js::CheckedUnwrap(sandboxArg)); + // CheckedUnwrapStatic is fine here, since we're checking for "is it a + // sandbox". + RootedObject sandbox(cx, js::CheckedUnwrapStatic(sandboxArg)); if (!sandbox || !IsSandbox(sandbox)) { return NS_ERROR_INVALID_ARG; } diff --git a/js/xpconnect/src/XPCCallContext.cpp b/js/xpconnect/src/XPCCallContext.cpp index 9e5a0400f87a..e5e442eb4b59 100644 --- a/js/xpconnect/src/XPCCallContext.cpp +++ b/js/xpconnect/src/XPCCallContext.cpp @@ -61,7 +61,8 @@ XPCCallContext::XPCCallContext( mTearOff = nullptr; - JSObject* unwrapped = js::CheckedUnwrap(obj, /* stopAtWindowProxy = */ false); + JSObject* unwrapped = + js::CheckedUnwrapDynamic(obj, cx, /* stopAtWindowProxy = */ false); if (!unwrapped) { JS_ReportErrorASCII(mJSContext, "Permission denied to call method on |this|"); diff --git a/js/xpconnect/src/XPCComponents.cpp b/js/xpconnect/src/XPCComponents.cpp index b2ae7051650f..bdd13fec2943 100644 --- a/js/xpconnect/src/XPCComponents.cpp +++ b/js/xpconnect/src/XPCComponents.cpp @@ -1529,7 +1529,8 @@ nsXPCComponents_Utils::GetSandboxMetadata(HandleValue sandboxVal, JSContext* cx, } RootedObject sandbox(cx, &sandboxVal.toObject()); - sandbox = js::CheckedUnwrap(sandbox); + // We only care about sandboxes here, so CheckedUnwrapStatic is fine. + sandbox = js::CheckedUnwrapStatic(sandbox); if (!sandbox || !xpc::IsSandbox(sandbox)) { return NS_ERROR_INVALID_ARG; } @@ -1546,7 +1547,8 @@ nsXPCComponents_Utils::SetSandboxMetadata(HandleValue sandboxVal, } RootedObject sandbox(cx, &sandboxVal.toObject()); - sandbox = js::CheckedUnwrap(sandbox); + // We only care about sandboxes here, so CheckedUnwrapStatic is fine. + sandbox = js::CheckedUnwrapStatic(sandbox); if (!sandbox || !xpc::IsSandbox(sandbox)) { return NS_ERROR_INVALID_ARG; } @@ -1838,7 +1840,10 @@ nsXPCComponents_Utils::IsProxy(HandleValue vobj, JSContext* cx, bool* rval) { } RootedObject obj(cx, &vobj.toObject()); - obj = js::CheckedUnwrap(obj, /* stopAtWindowProxy = */ false); + // We need to do a dynamic unwrap, because we apparently want to treat + // "failure to unwrap" differently from "not a proxy" (throw for the former, + // return false for the latter). + obj = js::CheckedUnwrapDynamic(obj, cx, /* stopAtWindowProxy = */ false); NS_ENSURE_TRUE(obj, NS_ERROR_FAILURE); *rval = js::IsScriptedProxy(obj); @@ -2317,7 +2322,8 @@ bool xpc::CloneInto(JSContext* aCx, HandleValue aValue, HandleValue aScope, } RootedObject scope(aCx, &aScope.toObject()); - scope = js::CheckedUnwrap(scope); + // The scope could be a Window, so we need to CheckedUnwrapDynamic. + scope = js::CheckedUnwrapDynamic(scope, aCx); if (!scope) { JS_ReportErrorASCII(aCx, "Permission denied to clone object into scope"); return false; @@ -2378,7 +2384,9 @@ nsXPCComponents_Utils::GetObjectPrincipal(HandleValue val, JSContext* cx, return NS_ERROR_INVALID_ARG; } RootedObject obj(cx, &val.toObject()); - obj = js::CheckedUnwrap(obj); + // We need to be able to unwrap to WindowProxy or Location here, so + // use CheckedUnwrapDynamic. + obj = js::CheckedUnwrapDynamic(obj, cx); MOZ_ASSERT(obj); nsCOMPtr prin = nsContentUtils::ObjectPrincipal(obj); @@ -2393,7 +2401,9 @@ nsXPCComponents_Utils::GetRealmLocation(HandleValue val, JSContext* cx, return NS_ERROR_INVALID_ARG; } RootedObject obj(cx, &val.toObject()); - obj = js::CheckedUnwrap(obj); + // We need to be able to unwrap to WindowProxy or Location here, so + // use CheckedUnwrapDynamic. + obj = js::CheckedUnwrapDynamic(obj, cx); MOZ_ASSERT(obj); result = xpc::RealmPrivate::Get(obj)->GetLocation(); diff --git a/js/xpconnect/src/XPCConvert.cpp b/js/xpconnect/src/XPCConvert.cpp index 0a1a5fa9f4f9..e055735764b7 100644 --- a/js/xpconnect/src/XPCConvert.cpp +++ b/js/xpconnect/src/XPCConvert.cpp @@ -1069,9 +1069,9 @@ bool XPCConvert::JSObject2NativeInterface(JSContext* cx, void** dest, // scope - see nsBindingManager::GetBindingImplementation. // // It's also very important that "inner" be rooted here. - RootedObject inner(RootingCx(), - js::CheckedUnwrap(src, - /* stopAtWindowProxy = */ false)); + RootedObject inner( + cx, js::CheckedUnwrapDynamic(src, cx, + /* stopAtWindowProxy = */ false)); if (!inner) { if (pErr) { *pErr = NS_ERROR_XPC_SECURITY_MANAGER_VETO; @@ -1278,12 +1278,14 @@ nsresult XPCConvert::JSValToXPCException(MutableHandleValue s, // is this really a native xpcom object with a wrapper? JSObject* unwrapped = - js::CheckedUnwrap(obj, /* stopAtWindowProxy = */ false); + js::CheckedUnwrapDynamic(obj, cx, /* stopAtWindowProxy = */ false); if (!unwrapped) { return NS_ERROR_XPC_SECURITY_MANAGER_VETO; } + // It's OK to use ReflectorToISupportsStatic, because we have already + // stripped off wrappers. if (nsCOMPtr supports = - UnwrapReflectorToISupports(unwrapped)) { + ReflectorToISupportsStatic(unwrapped)) { nsCOMPtr iface = do_QueryInterface(supports); if (iface) { // just pass through the exception (with extra ref and all) diff --git a/js/xpconnect/src/XPCJSContext.cpp b/js/xpconnect/src/XPCJSContext.cpp index cc9c358e46c7..69023ef6eb22 100644 --- a/js/xpconnect/src/XPCJSContext.cpp +++ b/js/xpconnect/src/XPCJSContext.cpp @@ -659,7 +659,8 @@ bool XPCJSContext::InterruptCallback(JSContext* cx) { return false; } if (proto && xpc::IsSandboxPrototypeProxy(proto) && - (proto = js::CheckedUnwrap(proto, /* stopAtWindowProxy = */ false))) { + (proto = js::CheckedUnwrapDynamic(proto, cx, + /* stopAtWindowProxy = */ false))) { win = WindowGlobalOrNull(proto); } } diff --git a/js/xpconnect/src/XPCJSID.cpp b/js/xpconnect/src/XPCJSID.cpp index cfd3da91c68f..e2627a2e9b15 100644 --- a/js/xpconnect/src/XPCJSID.cpp +++ b/js/xpconnect/src/XPCJSID.cpp @@ -147,7 +147,8 @@ static JSObject* GetIDPrototype(JSContext* aCx, const js::Class* aClass) { // Unwrap the given value to an object with the correct class, or nullptr. static JSObject* GetIDObject(HandleValue aVal, const Class* aClass) { if (aVal.isObject()) { - JSObject* obj = js::CheckedUnwrap(&aVal.toObject()); + // We care only about IID/CID objects here, so CheckedUnwrapStatic is fine. + JSObject* obj = js::CheckedUnwrapStatic(&aVal.toObject()); if (obj && js::GetObjectClass(obj) == aClass) { return obj; } @@ -168,8 +169,13 @@ Maybe JSValue2ID(JSContext* aCx, HandleValue aVal) { return Nothing(); } + // We only care about ID objects here, so CheckedUnwrapStatic is fine. + RootedObject obj(aCx, js::CheckedUnwrapStatic(&aVal.toObject())); + if (!obj) { + return Nothing(); + } + mozilla::Maybe id; - RootedObject obj(aCx, js::CheckedUnwrap(&aVal.toObject())); if (js::GetObjectClass(obj) == &sID_Class) { // Extract the raw bytes of the nsID from reserved slots. uint32_t rawid[] = {js::GetReservedSlot(obj, kID_Slot0).toPrivateUint32(), @@ -367,8 +373,11 @@ static nsresult FindObjectForHasInstance(JSContext* cx, HandleObject objArg, using namespace mozilla::jsipc; RootedObject obj(cx, objArg), proto(cx); while (true) { - // Try the object, or the wrappee if allowed. - JSObject* o = js::IsWrapper(obj) ? js::CheckedUnwrap(obj, false) : obj; + // Try the object, or the wrappee if allowed. We want CheckedUnwrapDynamic + // here, because we might in fact be looking for a Window. "cx" represents + // our current global. + JSObject* o = + js::IsWrapper(obj) ? js::CheckedUnwrapDynamic(obj, cx, false) : obj; if (o && (IS_WN_REFLECTOR(o) || IsDOMObject(o) || IsCPOW(o))) { target.set(o); return NS_OK; @@ -405,7 +414,8 @@ nsresult HasInstance(JSContext* cx, HandleObject objArg, const nsID* iid, return mozilla::jsipc::InstanceOf(obj, iid, bp); } - nsCOMPtr identity = UnwrapReflectorToISupports(obj); + // Need to unwrap Window correctly here, so use ReflectorToISupportsDynamic. + nsCOMPtr identity = ReflectorToISupportsDynamic(obj, cx); if (!identity) { return NS_OK; } diff --git a/js/xpconnect/src/XPCJSWeakReference.cpp b/js/xpconnect/src/XPCJSWeakReference.cpp index 2fb843148635..24563225874d 100644 --- a/js/xpconnect/src/XPCJSWeakReference.cpp +++ b/js/xpconnect/src/XPCJSWeakReference.cpp @@ -25,7 +25,7 @@ nsresult xpcJSWeakReference::Init(JSContext* cx, const JS::Value& object) { XPCCallContext ccx(cx); // See if the object is a wrapped native that supports weak references. - nsCOMPtr supports = xpc::UnwrapReflectorToISupports(obj); + nsCOMPtr supports = xpc::ReflectorToISupportsDynamic(obj, cx); nsCOMPtr supportsWeakRef = do_QueryInterface(supports); if (supportsWeakRef) { diff --git a/js/xpconnect/src/XPCVariant.cpp b/js/xpconnect/src/XPCVariant.cpp index 17a1d3517f91..f5c40b056897 100644 --- a/js/xpconnect/src/XPCVariant.cpp +++ b/js/xpconnect/src/XPCVariant.cpp @@ -44,10 +44,11 @@ XPCVariant::XPCVariant(JSContext* cx, const Value& aJSVal) mJSVal = JS::ObjectValue(*obj); JSObject* unwrapped = - js::CheckedUnwrap(obj, /* stopAtWindowProxy = */ false); + js::CheckedUnwrapDynamic(obj, cx, /* stopAtWindowProxy = */ false); mReturnRawObject = !(unwrapped && IS_WN_REFLECTOR(unwrapped)); - } else + } else { mReturnRawObject = false; + } } XPCTraceableVariant::~XPCTraceableVariant() { diff --git a/js/xpconnect/src/XPCWrappedNativeJSOps.cpp b/js/xpconnect/src/XPCWrappedNativeJSOps.cpp index 487cfba78d1c..50668b89f0ea 100644 --- a/js/xpconnect/src/XPCWrappedNativeJSOps.cpp +++ b/js/xpconnect/src/XPCWrappedNativeJSOps.cpp @@ -703,7 +703,7 @@ bool XPC_WN_MaybeResolvingDeletePropertyStub(JSContext* cx, HandleObject obj, // macro fun! #define PRE_HELPER_STUB \ /* It's very important for "unwrapped" to be rooted here. */ \ - RootedObject unwrapped(cx, js::CheckedUnwrap(obj, false)); \ + RootedObject unwrapped(cx, js::CheckedUnwrapDynamic(obj, cx, false)); \ if (!unwrapped) { \ JS_ReportErrorASCII(cx, "Permission denied to operate on object."); \ return false; \ diff --git a/js/xpconnect/src/nsXPConnect.cpp b/js/xpconnect/src/nsXPConnect.cpp index a9c2b01a0357..938a1c4a4339 100644 --- a/js/xpconnect/src/nsXPConnect.cpp +++ b/js/xpconnect/src/nsXPConnect.cpp @@ -682,7 +682,8 @@ nsXPConnect::GetWrappedNativeOfJSObject(JSContext* aJSContext, MOZ_ASSERT(_retval, "bad param"); RootedObject aJSObj(aJSContext, aJSObjArg); - aJSObj = js::CheckedUnwrap(aJSObj, /* stopAtWindowProxy = */ false); + aJSObj = js::CheckedUnwrapDynamic(aJSObj, aJSContext, + /* stopAtWindowProxy = */ false); if (!aJSObj || !IS_WN_REFLECTOR(aJSObj)) { *_retval = nullptr; return NS_ERROR_FAILURE; @@ -693,10 +694,7 @@ nsXPConnect::GetWrappedNativeOfJSObject(JSContext* aJSContext, return NS_OK; } -already_AddRefed xpc::UnwrapReflectorToISupports( - JSObject* reflector) { - // Unwrap security wrappers, if allowed. - reflector = js::CheckedUnwrap(reflector, /* stopAtWindowProxy = */ false); +static already_AddRefed ReflectorToISupports(JSObject* reflector) { if (!reflector) { return nullptr; } @@ -719,6 +717,20 @@ already_AddRefed xpc::UnwrapReflectorToISupports( return canonical.forget(); } +already_AddRefed xpc::ReflectorToISupportsStatic( + JSObject* reflector) { + // Unwrap security wrappers, if allowed. + return ReflectorToISupports(js::CheckedUnwrapStatic(reflector)); +} + +already_AddRefed xpc::ReflectorToISupportsDynamic( + JSObject* reflector, JSContext* cx) { + // Unwrap security wrappers, if allowed. + return ReflectorToISupports( + js::CheckedUnwrapDynamic(reflector, cx, + /* stopAtWindowProxy = */ false)); +} + NS_IMETHODIMP nsXPConnect::CreateSandbox(JSContext* cx, nsIPrincipal* principal, JSObject** _retval) { diff --git a/js/xpconnect/src/xpcpublic.h b/js/xpconnect/src/xpcpublic.h index 240dff6cb566..1ff340ea5ee7 100644 --- a/js/xpconnect/src/xpcpublic.h +++ b/js/xpconnect/src/xpcpublic.h @@ -139,7 +139,11 @@ void ClearContentXBLScope(JSObject* global); bool IsSandboxPrototypeProxy(JSObject* obj); -bool IsReflector(JSObject* obj); +// The JSContext argument represents the Realm that's asking the question. This +// is needed to properly answer without exposing information unnecessarily +// from behind security wrappers. There will be no exceptions thrown on this +// JSContext. +bool IsReflector(JSObject* obj, JSContext* cx); bool IsXrayWrapper(JSObject* obj); @@ -460,9 +464,24 @@ bool Throw(JSContext* cx, nsresult rv); /** * Returns the nsISupports native behind a given reflector (either DOM or - * XPCWN). + * XPCWN). If a non-reflector object is passed in, null will be returned. + * + * This function will not correctly handle Window or Location objects behind + * cross-compartment wrappers: it will return null. If you care about getting + * non-null for Window or Location, use ReflectorToISupportsDynamic. */ -already_AddRefed UnwrapReflectorToISupports(JSObject* reflector); +already_AddRefed ReflectorToISupportsStatic(JSObject* reflector); + +/** + * Returns the nsISupports native behind a given reflector (either DOM or + * XPCWN). If a non-reflector object is passed in, null will be returned. + * + * The JSContext argument represents the Realm that's asking for the + * nsISupports. This is needed to properly handle Window and Location objects, + * which do dynamic security checks. + */ +already_AddRefed ReflectorToISupportsDynamic(JSObject* reflector, + JSContext* cx); /** * Singleton scopes for stuff that really doesn't fit anywhere else. diff --git a/js/xpconnect/wrappers/XrayWrapper.cpp b/js/xpconnect/wrappers/XrayWrapper.cpp index c35cd051946f..de6dbe7d5ec6 100644 --- a/js/xpconnect/wrappers/XrayWrapper.cpp +++ b/js/xpconnect/wrappers/XrayWrapper.cpp @@ -33,7 +33,7 @@ using namespace JS; using namespace mozilla; using js::BaseProxyHandler; -using js::CheckedUnwrap; +using js::CheckedUnwrapStatic; using js::IsCrossCompartmentWrapper; using js::UncheckedUnwrap; using js::Wrapper; @@ -1934,13 +1934,15 @@ static bool RecreateLostWaivers(JSContext* cx, const PropertyDescriptor* orig, wrapped.value().set(ObjectValue(*rewaived)); } if (getterWasWaived && !IsCrossCompartmentWrapper(wrapped.getterObject())) { - MOZ_ASSERT(CheckedUnwrap(wrapped.getterObject())); + // We can't end up with WindowProxy or Location as getters. + MOZ_ASSERT(CheckedUnwrapStatic(wrapped.getterObject())); rewaived = WrapperFactory::WaiveXray(cx, wrapped.getterObject()); NS_ENSURE_TRUE(rewaived, false); wrapped.setGetterObject(rewaived); } if (setterWasWaived && !IsCrossCompartmentWrapper(wrapped.setterObject())) { - MOZ_ASSERT(CheckedUnwrap(wrapped.setterObject())); + // We can't end up with WindowProxy or Location as setters. + MOZ_ASSERT(CheckedUnwrapStatic(wrapped.setterObject())); rewaived = WrapperFactory::WaiveXray(cx, wrapped.setterObject()); NS_ENSURE_TRUE(rewaived, false); wrapped.setSetterObject(rewaived); diff --git a/toolkit/components/telemetry/core/Stopwatch.cpp b/toolkit/components/telemetry/core/Stopwatch.cpp index e6fb82187ffa..b4f8dafa0104 100644 --- a/toolkit/components/telemetry/core/Stopwatch.cpp +++ b/toolkit/components/telemetry/core/Stopwatch.cpp @@ -25,7 +25,8 @@ using mozilla::dom::AutoJSAPI; static inline nsQueryObject do_QueryReflector( JSObject* aReflector) { - nsCOMPtr reflector = xpc::UnwrapReflectorToISupports(aReflector); + // None of the types we query to are implemented by Window or Location. + nsCOMPtr reflector = xpc::ReflectorToISupportsStatic(aReflector); return do_QueryObject(reflector); }