From be0882e3a19bdd9d7effc31bbcfe39f1f1dcc5a6 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Mon, 20 May 2013 08:40:06 -0400 Subject: [PATCH] Bug 873735 part 1. Fix the more or less mechanical browser rooting hazards. r=terrence --- caps/include/nsScriptSecurityManager.h | 5 +++-- caps/src/nsScriptSecurityManager.cpp | 14 ++++++++------ content/base/src/nsFrameLoader.cpp | 2 +- content/base/src/nsFrameMessageManager.cpp | 8 ++++---- content/base/src/nsFrameMessageManager.h | 2 +- content/base/src/nsInProcessTabChildGlobal.cpp | 5 +++-- content/xbl/src/nsXBLProtoImplMethod.cpp | 6 ++++-- content/xbl/src/nsXBLProtoImplProperty.cpp | 6 ++++-- content/xbl/src/nsXBLSerialize.cpp | 5 ++--- content/xbl/src/nsXBLSerialize.h | 2 +- content/xul/content/src/nsXULElement.cpp | 4 ++-- content/xul/content/src/nsXULElement.h | 8 ++++++-- content/xul/document/src/XULDocument.cpp | 6 +++--- content/xul/document/src/XULDocument.h | 3 ++- .../xul/document/src/nsXULPrototypeCache.cpp | 3 ++- content/xul/document/src/nsXULPrototypeCache.h | 2 +- dom/ipc/ContentChild.cpp | 2 +- dom/ipc/ContentParent.cpp | 6 +++--- dom/ipc/TabChild.cpp | 4 ++-- dom/ipc/TabParent.cpp | 2 +- js/xpconnect/public/nsTArrayHelpers.h | 17 ++++++++++------- 21 files changed, 64 insertions(+), 48 deletions(-) diff --git a/caps/include/nsScriptSecurityManager.h b/caps/include/nsScriptSecurityManager.h index 07f947b3717..add84f569d3 100644 --- a/caps/include/nsScriptSecurityManager.h +++ b/caps/include/nsScriptSecurityManager.h @@ -383,10 +383,11 @@ private: // Returns null if a principal cannot be found; generally callers // should error out at that point. - static nsIPrincipal* doGetObjectPrincipal(JSObject *obj); + static nsIPrincipal* doGetObjectPrincipal(JS::Handle obj); #ifdef DEBUG static nsIPrincipal* - old_doGetObjectPrincipal(JSObject *obj, bool aAllowShortCircuit = true); + old_doGetObjectPrincipal(JS::Handle obj, + bool aAllowShortCircuit = true); #endif // Returns null if a principal cannot be found. Note that rv can be NS_OK diff --git a/caps/src/nsScriptSecurityManager.cpp b/caps/src/nsScriptSecurityManager.cpp index a71d31f8101..10c23591921 100644 --- a/caps/src/nsScriptSecurityManager.cpp +++ b/caps/src/nsScriptSecurityManager.cpp @@ -1633,7 +1633,7 @@ nsScriptSecurityManager::CheckFunctionAccess(JSContext *aCx, void *aFunObj, /* ** Get origin of subject and object and compare. */ - JSObject* obj = (JSObject*)aTargetObj; + JS::Rooted obj(aCx, (JSObject*)aTargetObj); nsIPrincipal* object = doGetObjectPrincipal(obj); if (!object) @@ -2023,7 +2023,8 @@ NS_IMETHODIMP nsScriptSecurityManager::GetObjectPrincipal(JSContext *aCx, JSObject *aObj, nsIPrincipal **result) { - *result = doGetObjectPrincipal(aObj); + JS::Rooted obj(aCx, aObj); + *result = doGetObjectPrincipal(obj); if (!*result) return NS_ERROR_FAILURE; NS_ADDREF(*result); @@ -2032,7 +2033,7 @@ nsScriptSecurityManager::GetObjectPrincipal(JSContext *aCx, JSObject *aObj, // static nsIPrincipal* -nsScriptSecurityManager::doGetObjectPrincipal(JSObject *aObj) +nsScriptSecurityManager::doGetObjectPrincipal(JS::Handle aObj) { JSCompartment *compartment = js::GetObjectCompartment(aObj); JSPrincipals *principals = JS_GetCompartmentPrincipals(compartment); @@ -2052,14 +2053,15 @@ nsScriptSecurityManager::doGetObjectPrincipal(JSObject *aObj) #ifdef DEBUG // static nsIPrincipal* -nsScriptSecurityManager::old_doGetObjectPrincipal(JSObject *aObj, +nsScriptSecurityManager::old_doGetObjectPrincipal(JS::Handle aObj, bool aAllowShortCircuit) { NS_ASSERTION(aObj, "Bad call to doGetObjectPrincipal()!"); nsIPrincipal* result = nullptr; - JS::RootedObject obj(sXPConnect->GetCurrentJSContext(), aObj); - JSObject* origObj = obj; + JSContext* cx = sXPConnect->GetCurrentJSContext(); + JS::RootedObject obj(cx, aObj); + JS::RootedObject origObj(cx, obj); js::Class *jsClass = js::GetObjectClass(obj); // A common case seen in this code is that we enter this function diff --git a/content/base/src/nsFrameLoader.cpp b/content/base/src/nsFrameLoader.cpp index a810fb577da..e044e38c88b 100644 --- a/content/base/src/nsFrameLoader.cpp +++ b/content/base/src/nsFrameLoader.cpp @@ -2204,7 +2204,7 @@ public: nsRefPtr mm = tabChild->GetInnerManager(); mm->ReceiveMessage(static_cast(tabChild), mMessage, - false, &data, nullptr, nullptr, nullptr); + false, &data, JS::NullPtr(), nullptr, nullptr); } return NS_OK; } diff --git a/content/base/src/nsFrameMessageManager.cpp b/content/base/src/nsFrameMessageManager.cpp index 8751fc352d8..1392477c613 100644 --- a/content/base/src/nsFrameMessageManager.cpp +++ b/content/base/src/nsFrameMessageManager.cpp @@ -626,7 +626,7 @@ nsFrameMessageManager::ReceiveMessage(nsISupports* aTarget, const nsAString& aMessage, bool aSync, const StructuredCloneData* aCloneData, - JSObject* aObjectsArray, + JS::Handle aObjectsArray, InfallibleTArray* aJSONRetVal, JSContext* aContext) { @@ -1206,7 +1206,7 @@ public: nsRefPtr ppm = nsFrameMessageManager::sChildProcessManager; ppm->ReceiveMessage(static_cast(ppm.get()), mMessage, - false, &data, nullptr, nullptr, nullptr); + false, &data, JS::NullPtr(), nullptr, nullptr); } return NS_OK; } @@ -1336,7 +1336,7 @@ public: nsRefPtr ppm = nsFrameMessageManager::sSameProcessParentManager; ppm->ReceiveMessage(static_cast(ppm.get()), - mMessage, false, &data, nullptr, nullptr, nullptr); + mMessage, false, &data, JS::NullPtr(), nullptr, nullptr); } return NS_OK; } @@ -1376,7 +1376,7 @@ public: if (nsFrameMessageManager::sSameProcessParentManager) { nsRefPtr ppm = nsFrameMessageManager::sSameProcessParentManager; ppm->ReceiveMessage(static_cast(ppm.get()), aMessage, - true, &aData, nullptr, aJSONRetVal); + true, &aData, JS::NullPtr(), aJSONRetVal); } return true; } diff --git a/content/base/src/nsFrameMessageManager.h b/content/base/src/nsFrameMessageManager.h index 5ee3ac15b88..2df5d5a0009 100644 --- a/content/base/src/nsFrameMessageManager.h +++ b/content/base/src/nsFrameMessageManager.h @@ -182,7 +182,7 @@ public: nsresult ReceiveMessage(nsISupports* aTarget, const nsAString& aMessage, bool aSync, const StructuredCloneData* aCloneData, - JSObject* aObjectsArray, + JS::Handle aObjectsArray, InfallibleTArray* aJSONRetVal, JSContext* aContext = nullptr); diff --git a/content/base/src/nsInProcessTabChildGlobal.cpp b/content/base/src/nsInProcessTabChildGlobal.cpp index 517e076976c..0769290b263 100644 --- a/content/base/src/nsInProcessTabChildGlobal.cpp +++ b/content/base/src/nsInProcessTabChildGlobal.cpp @@ -38,7 +38,8 @@ nsInProcessTabChildGlobal::DoSendSyncMessage(const nsAString& aMessage, } if (mChromeMessageManager) { nsRefPtr mm = mChromeMessageManager; - mm->ReceiveMessage(mOwner, aMessage, true, &aData, nullptr, aJSONRetVal); + mm->ReceiveMessage(mOwner, aMessage, true, &aData, JS::NullPtr(), + aJSONRetVal); } return true; } @@ -73,7 +74,7 @@ public: nsRefPtr mm = mTabChild->mChromeMessageManager; mm->ReceiveMessage(mTabChild->mOwner, mMessage, false, &data, - nullptr, nullptr, nullptr); + JS::NullPtr(), nullptr, nullptr); } return NS_OK; } diff --git a/content/xbl/src/nsXBLProtoImplMethod.cpp b/content/xbl/src/nsXBLProtoImplMethod.cpp index 902dcc20291..f3d701de086 100644 --- a/content/xbl/src/nsXBLProtoImplMethod.cpp +++ b/content/xbl/src/nsXBLProtoImplMethod.cpp @@ -264,7 +264,8 @@ nsXBLProtoImplMethod::Write(nsIScriptContext* aContext, rv = aStream->WriteWStringZ(mName); NS_ENSURE_SUCCESS(rv, rv); - return XBL_SerializeFunction(aContext, aStream, mJSMethodObject); + return XBL_SerializeFunction(aContext, aStream, + JS::Handle::fromMarkedLocation(&mJSMethodObject)); } return NS_OK; @@ -369,7 +370,8 @@ nsXBLProtoImplAnonymousMethod::Write(nsIScriptContext* aContext, nsresult rv = aStream->Write8(aType); NS_ENSURE_SUCCESS(rv, rv); - rv = XBL_SerializeFunction(aContext, aStream, mJSMethodObject); + rv = XBL_SerializeFunction(aContext, aStream, + JS::Handle::fromMarkedLocation(&mJSMethodObject)); NS_ENSURE_SUCCESS(rv, rv); } diff --git a/content/xbl/src/nsXBLProtoImplProperty.cpp b/content/xbl/src/nsXBLProtoImplProperty.cpp index 3a1198b390e..af29e0adfa1 100644 --- a/content/xbl/src/nsXBLProtoImplProperty.cpp +++ b/content/xbl/src/nsXBLProtoImplProperty.cpp @@ -373,12 +373,14 @@ nsXBLProtoImplProperty::Write(nsIScriptContext* aContext, NS_ENSURE_SUCCESS(rv, rv); if (mJSAttributes & JSPROP_GETTER) { - rv = XBL_SerializeFunction(aContext, aStream, mJSGetterObject); + rv = XBL_SerializeFunction(aContext, aStream, + JS::Handle::fromMarkedLocation(&mJSGetterObject)); NS_ENSURE_SUCCESS(rv, rv); } if (mJSAttributes & JSPROP_SETTER) { - rv = XBL_SerializeFunction(aContext, aStream, mJSSetterObject); + rv = XBL_SerializeFunction(aContext, aStream, + JS::Handle::fromMarkedLocation(&mJSSetterObject)); NS_ENSURE_SUCCESS(rv, rv); } diff --git a/content/xbl/src/nsXBLSerialize.cpp b/content/xbl/src/nsXBLSerialize.cpp index 6afc3a8107a..efef3a42331 100644 --- a/content/xbl/src/nsXBLSerialize.cpp +++ b/content/xbl/src/nsXBLSerialize.cpp @@ -13,11 +13,10 @@ using namespace mozilla; nsresult XBL_SerializeFunction(nsIScriptContext* aContext, nsIObjectOutputStream* aStream, - JSObject* aFunctionObject) + JS::Handle aFunction) { AutoPushJSContext cx(aContext->GetNativeContext()); - JS::RootedObject function(cx, aFunctionObject); - return nsContentUtils::XPConnect()->WriteFunction(aStream, cx, function); + return nsContentUtils::XPConnect()->WriteFunction(aStream, cx, aFunction); } nsresult diff --git a/content/xbl/src/nsXBLSerialize.h b/content/xbl/src/nsXBLSerialize.h index dd4dbfccc78..5263289bf7a 100644 --- a/content/xbl/src/nsXBLSerialize.h +++ b/content/xbl/src/nsXBLSerialize.h @@ -79,7 +79,7 @@ PR_STATIC_ASSERT(XBLBinding_Serialize_CustomNamespace >= kNameSpaceID_LastBuilti nsresult XBL_SerializeFunction(nsIScriptContext* aContext, nsIObjectOutputStream* aStream, - JSObject* aFunctionObject); + JS::Handle aFunctionObject); nsresult XBL_DeserializeFunction(nsIScriptContext* aContext, diff --git a/content/xul/content/src/nsXULElement.cpp b/content/xul/content/src/nsXULElement.cpp index 9dab14522f1..bbece95c99d 100644 --- a/content/xul/content/src/nsXULElement.cpp +++ b/content/xul/content/src/nsXULElement.cpp @@ -2526,7 +2526,7 @@ nsXULPrototypeScript::DeserializeOutOfLine(nsIObjectInputStream* aInput, bool isChrome = false; mSrcURI->SchemeIs("chrome", &isChrome); if (isChrome) - cache->PutScript(mSrcURI, mScriptObject); + cache->PutScript(mSrcURI, GetScriptObject()); } cache->FinishInputStream(mSrcURI); } else { @@ -2628,9 +2628,9 @@ nsXULPrototypeScript::Set(JSScript* aObject) return; } + mScriptObject = aObject; nsContentUtils::HoldJSObjects( this, NS_CYCLE_COLLECTION_PARTICIPANT(nsXULPrototypeNode)); - mScriptObject = aObject; } //---------------------------------------------------------------------- diff --git a/content/xul/content/src/nsXULElement.h b/content/xul/content/src/nsXULElement.h index 9fc895de55e..16b927cf449 100644 --- a/content/xul/content/src/nsXULElement.h +++ b/content/xul/content/src/nsXULElement.h @@ -236,9 +236,13 @@ public: void Set(JSScript* aObject); - JSScript *GetScriptObject() + // It's safe to return a handle because we trace mScriptObject, no one ever + // uses the handle (or the script object) past the point at which the + // nsXULPrototypeScript dies, and we can't get memmoved so the + // &mScriptObject pointer can't go stale. + JS::Handle GetScriptObject() { - return mScriptObject; + return JS::Handle::fromMarkedLocation(&mScriptObject); } void TraceScriptObject(JSTracer* aTrc) diff --git a/content/xul/document/src/XULDocument.cpp b/content/xul/document/src/XULDocument.cpp index d314bfe1546..d3e62d3fd37 100644 --- a/content/xul/document/src/XULDocument.cpp +++ b/content/xul/document/src/XULDocument.cpp @@ -3644,7 +3644,8 @@ XULDocument::OnStreamComplete(nsIStreamLoader* aLoader, nsresult -XULDocument::ExecuteScript(nsIScriptContext * aContext, JSScript* aScriptObject) +XULDocument::ExecuteScript(nsIScriptContext * aContext, + JS::Handle aScriptObject) { NS_PRECONDITION(aScriptObject != nullptr && aContext != nullptr, "null ptr"); if (! aScriptObject || ! aContext) @@ -3653,9 +3654,8 @@ XULDocument::ExecuteScript(nsIScriptContext * aContext, JSScript* aScriptObject) NS_ENSURE_TRUE(mScriptGlobalObject, NS_ERROR_NOT_INITIALIZED); // Execute the precompiled script with the given version - JS::Rooted script(aContext->GetNativeContext(), aScriptObject); JSObject* global = mScriptGlobalObject->GetGlobalJSObject(); - return aContext->ExecuteScript(script, global); + return aContext->ExecuteScript(aScriptObject, global); } nsresult diff --git a/content/xul/document/src/XULDocument.h b/content/xul/document/src/XULDocument.h index a7354adcf60..1ec16af835c 100644 --- a/content/xul/document/src/XULDocument.h +++ b/content/xul/document/src/XULDocument.h @@ -403,7 +403,8 @@ protected: * Execute the precompiled script object scoped by this XUL document's * containing window object, and using its associated script context. */ - nsresult ExecuteScript(nsIScriptContext *aContext, JSScript* aScriptObject); + nsresult ExecuteScript(nsIScriptContext *aContext, + JS::Handle aScriptObject); /** * Helper method for the above that uses aScript to find the appropriate diff --git a/content/xul/document/src/nsXULPrototypeCache.cpp b/content/xul/document/src/nsXULPrototypeCache.cpp index 92ae12c24f3..75078fbc3a9 100644 --- a/content/xul/document/src/nsXULPrototypeCache.cpp +++ b/content/xul/document/src/nsXULPrototypeCache.cpp @@ -205,7 +205,8 @@ nsXULPrototypeCache::GetScript(nsIURI* aURI) } nsresult -nsXULPrototypeCache::PutScript(nsIURI* aURI, JSScript* aScriptObject) +nsXULPrototypeCache::PutScript(nsIURI* aURI, + JS::Handle aScriptObject) { CacheScriptEntry existingEntry; if (mScriptTable.Get(aURI, &existingEntry)) { diff --git a/content/xul/document/src/nsXULPrototypeCache.h b/content/xul/document/src/nsXULPrototypeCache.h index 8ec5e475956..c7b670a11a0 100644 --- a/content/xul/document/src/nsXULPrototypeCache.h +++ b/content/xul/document/src/nsXULPrototypeCache.h @@ -69,7 +69,7 @@ public: nsresult PutPrototype(nsXULPrototypeDocument* aDocument); JSScript* GetScript(nsIURI* aURI); - nsresult PutScript(nsIURI* aURI, JSScript* aScriptObject); + nsresult PutScript(nsIURI* aURI, JS::Handle aScriptObject); nsXBLDocumentInfo* GetXBLDocumentInfo(nsIURI* aURL) { return mXBLDocTable.GetWeak(aURL); diff --git a/dom/ipc/ContentChild.cpp b/dom/ipc/ContentChild.cpp index 18fa5637c4c..9c542e7dfde 100644 --- a/dom/ipc/ContentChild.cpp +++ b/dom/ipc/ContentChild.cpp @@ -1026,7 +1026,7 @@ ContentChild::RecvAsyncMessage(const nsString& aMsg, if (cpm) { StructuredCloneData cloneData = ipc::UnpackClonedMessageDataForChild(aData); cpm->ReceiveMessage(static_cast(cpm.get()), - aMsg, false, &cloneData, nullptr, nullptr); + aMsg, false, &cloneData, JS::NullPtr(), nullptr); } return true; } diff --git a/dom/ipc/ContentParent.cpp b/dom/ipc/ContentParent.cpp index 7192cbdfca8..32161ffcc2d 100644 --- a/dom/ipc/ContentParent.cpp +++ b/dom/ipc/ContentParent.cpp @@ -866,7 +866,7 @@ ContentParent::ActorDestroy(ActorDestroyReason why) if (ppm) { ppm->ReceiveMessage(static_cast(ppm.get()), CHILD_PROCESS_SHUTDOWN_MESSAGE, false, - nullptr, nullptr, nullptr); + nullptr, JS::NullPtr(), nullptr); } nsCOMPtr kungFuDeathGrip(static_cast(this)); @@ -2331,7 +2331,7 @@ ContentParent::RecvSyncMessage(const nsString& aMsg, if (ppm) { StructuredCloneData cloneData = ipc::UnpackClonedMessageDataForParent(aData); ppm->ReceiveMessage(static_cast(ppm.get()), - aMsg, true, &cloneData, nullptr, aRetvals); + aMsg, true, &cloneData, JS::NullPtr(), aRetvals); } return true; } @@ -2344,7 +2344,7 @@ ContentParent::RecvAsyncMessage(const nsString& aMsg, if (ppm) { StructuredCloneData cloneData = ipc::UnpackClonedMessageDataForParent(aData); ppm->ReceiveMessage(static_cast(ppm.get()), - aMsg, false, &cloneData, nullptr, nullptr); + aMsg, false, &cloneData, JS::NullPtr(), nullptr); } return true; } diff --git a/dom/ipc/TabChild.cpp b/dom/ipc/TabChild.cpp index 22e9b39470d..41dbce5a304 100644 --- a/dom/ipc/TabChild.cpp +++ b/dom/ipc/TabChild.cpp @@ -1436,7 +1436,7 @@ TabChild::DispatchMessageManagerMessage(const nsAString& aMessageName, nsRefPtr mm = static_cast(mTabChildGlobal->mMessageManager.get()); mm->ReceiveMessage(static_cast(mTabChildGlobal), - aMessageName, false, &cloneData, nullptr, nullptr); + aMessageName, false, &cloneData, JS::NullPtr(), nullptr); } static void @@ -1974,7 +1974,7 @@ TabChild::RecvAsyncMessage(const nsString& aMessage, nsRefPtr mm = static_cast(mTabChildGlobal->mMessageManager.get()); mm->ReceiveMessage(static_cast(mTabChildGlobal), - aMessage, false, &cloneData, nullptr, nullptr); + aMessage, false, &cloneData, JS::NullPtr(), nullptr); } return true; } diff --git a/dom/ipc/TabParent.cpp b/dom/ipc/TabParent.cpp index a665a0d028e..a6d56d0396b 100644 --- a/dom/ipc/TabParent.cpp +++ b/dom/ipc/TabParent.cpp @@ -1102,7 +1102,7 @@ TabParent::ReceiveMessage(const nsString& aMessage, uint32_t len = 0; //TODO: obtain a real value in bug 572685 // Because we want JS messages to have always the same properties, // create array even if len == 0. - JSObject* objectsArray = JS_NewArrayObject(ctx, len, NULL); + JS::Rooted objectsArray(ctx, JS_NewArrayObject(ctx, len, NULL)); if (!objectsArray) { return false; } diff --git a/js/xpconnect/public/nsTArrayHelpers.h b/js/xpconnect/public/nsTArrayHelpers.h index 36c9cc41716..d16ff1f6760 100644 --- a/js/xpconnect/public/nsTArrayHelpers.h +++ b/js/xpconnect/public/nsTArrayHelpers.h @@ -13,7 +13,8 @@ nsTArrayToJSArray(JSContext* aCx, const nsTArray& aSourceArray, MOZ_ASSERT(aCx); JSAutoRequest ar(aCx); - JSObject* arrayObj = JS_NewArrayObject(aCx, aSourceArray.Length(), nullptr); + JS::Rooted arrayObj(aCx, + JS_NewArrayObject(aCx, aSourceArray.Length(), nullptr)); if (!arrayObj) { NS_WARNING("JS_NewArrayObject failed!"); return NS_ERROR_OUT_OF_MEMORY; @@ -27,11 +28,12 @@ nsTArrayToJSArray(JSContext* aCx, const nsTArray& aSourceArray, nsresult rv = aSourceArray[index]->QueryInterface(NS_GET_IID(nsISupports), getter_AddRefs(obj)); NS_ENSURE_SUCCESS(rv, rv); - jsval wrappedVal; - rv = nsContentUtils::WrapNative(aCx, global, obj, &wrappedVal, nullptr, true); + JS::Rooted wrappedVal(aCx); + rv = nsContentUtils::WrapNative(aCx, global, obj, wrappedVal.address(), + nullptr, true); NS_ENSURE_SUCCESS(rv, rv); - if (!JS_SetElement(aCx, arrayObj, index, &wrappedVal)) { + if (!JS_SetElement(aCx, arrayObj, index, wrappedVal.address())) { NS_WARNING("JS_SetElement failed!"); return NS_ERROR_FAILURE; } @@ -55,7 +57,8 @@ nsTArrayToJSArray(JSContext* aCx, MOZ_ASSERT(aCx); JSAutoRequest ar(aCx); - JSObject* arrayObj = JS_NewArrayObject(aCx, aSourceArray.Length(), nullptr); + JS::Rooted arrayObj(aCx, + JS_NewArrayObject(aCx, aSourceArray.Length(), nullptr)); if (!arrayObj) { NS_WARNING("JS_NewArrayObject failed!"); return NS_ERROR_OUT_OF_MEMORY; @@ -70,9 +73,9 @@ nsTArrayToJSArray(JSContext* aCx, return NS_ERROR_OUT_OF_MEMORY; } - jsval wrappedVal = STRING_TO_JSVAL(s); + JS::Rooted wrappedVal(aCx, STRING_TO_JSVAL(s)); - if (!JS_SetElement(aCx, arrayObj, index, &wrappedVal)) { + if (!JS_SetElement(aCx, arrayObj, index, wrappedVal.address())) { NS_WARNING("JS_SetElement failed!"); return NS_ERROR_FAILURE; }