Bug 1297300 - Add missing checks to GetSpec() calls in caps/ and js/. r=mrbkap.
authorNicholas Nethercote <nnethercote@mozilla.com>
Tue, 30 Aug 2016 14:22:04 +1000
changeset 412149 ee83d585ff9db5e419c9ed5bf21884c8689df19a
parent 412148 fbe1cc85a7e6b96ba60f8b61f5721f18ab2ee2d6
child 412150 d6663520be22a4f183f5b2e227bbda0f178a101b
push id29058
push userjdescottes@mozilla.com
push dateFri, 09 Sep 2016 11:47:42 +0000
reviewersmrbkap
bugs1297300
milestone51.0a1
Bug 1297300 - Add missing checks to GetSpec() calls in caps/ and js/. r=mrbkap. This required making GetScriptLocation() fallible.
caps/nsJSPrincipals.cpp
caps/nsJSPrincipals.h
caps/nsNullPrincipal.cpp
caps/nsNullPrincipal.h
caps/nsNullPrincipalURI.cpp
caps/nsPrincipal.cpp
caps/nsPrincipal.h
caps/nsSystemPrincipal.cpp
caps/nsSystemPrincipal.h
js/xpconnect/loader/mozJSSubScriptLoader.cpp
js/xpconnect/src/Sandbox.cpp
js/xpconnect/src/XPCJSRuntime.cpp
--- a/caps/nsJSPrincipals.cpp
+++ b/caps/nsJSPrincipals.cpp
@@ -83,18 +83,19 @@ nsJSPrincipals::Destroy(JSPrincipals *js
 #ifdef DEBUG
 
 // Defined here so one can do principals->dump() in the debugger
 JS_PUBLIC_API(void)
 JSPrincipals::dump()
 {
     if (debugToken == nsJSPrincipals::DEBUG_TOKEN) {
       nsAutoCString str;
-      static_cast<nsJSPrincipals *>(this)->GetScriptLocation(str);
-      fprintf(stderr, "nsIPrincipal (%p) = %s\n", static_cast<void*>(this), str.get());
+      nsresult rv = static_cast<nsJSPrincipals *>(this)->GetScriptLocation(str);
+      fprintf(stderr, "nsIPrincipal (%p) = %s\n", static_cast<void*>(this),
+              NS_SUCCEEDED(rv) ? str.get() : "(unknown)");
     } else if (debugToken == dom::workers::kJSPrincipalsDebugToken) {
         fprintf(stderr, "Web Worker principal singleton (%p)\n", this);
     } else {
         fprintf(stderr,
                 "!!! JSPrincipals (%p) is not nsJSPrincipals instance - bad token: "
                 "actual=0x%x expected=0x%x\n",
                 this, unsigned(debugToken), unsigned(nsJSPrincipals::DEBUG_TOKEN));
     }
--- a/caps/nsJSPrincipals.h
+++ b/caps/nsJSPrincipals.h
@@ -48,17 +48,17 @@ public:
   nsJSPrincipals() {
     refcount = 0;
     setDebugToken(DEBUG_TOKEN);
   }
 
   /**
    * Return a string that can be used as JS script filename in error reports.
    */
-  virtual void GetScriptLocation(nsACString &aStr) = 0;
+  virtual nsresult GetScriptLocation(nsACString &aStr) = 0;
   static const uint32_t DEBUG_TOKEN = 0x0bf41760;
 
 protected:
   virtual ~nsJSPrincipals() {
     setDebugToken(0);
   }
 
 };
--- a/caps/nsNullPrincipal.cpp
+++ b/caps/nsNullPrincipal.cpp
@@ -73,20 +73,20 @@ nsNullPrincipal::Init(const PrincipalOri
   mOriginAttributes = aOriginAttributes;
 
   mURI = nsNullPrincipalURI::Create();
   NS_ENSURE_TRUE(mURI, NS_ERROR_NOT_AVAILABLE);
 
   return NS_OK;
 }
 
-void
+nsresult
 nsNullPrincipal::GetScriptLocation(nsACString &aStr)
 {
-  mURI->GetSpec(aStr);
+  return mURI->GetSpec(aStr);
 }
 
 /**
  * nsIPrincipal implementation
  */
 
 NS_IMETHODIMP
 nsNullPrincipal::GetHashValue(uint32_t *aResult)
--- a/caps/nsNullPrincipal.h
+++ b/caps/nsNullPrincipal.h
@@ -52,17 +52,17 @@ public:
 
   static already_AddRefed<nsNullPrincipal> CreateWithInheritedAttributes(nsIDocShell* aDocShell);
 
   static already_AddRefed<nsNullPrincipal>
   Create(const mozilla::PrincipalOriginAttributes& aOriginAttributes = mozilla::PrincipalOriginAttributes());
 
   nsresult Init(const mozilla::PrincipalOriginAttributes& aOriginAttributes = mozilla::PrincipalOriginAttributes());
 
-  virtual void GetScriptLocation(nsACString &aStr) override;
+  virtual nsresult GetScriptLocation(nsACString &aStr) override;
 
   PrincipalKind Kind() override { return eNullPrincipal; }
 
  protected:
   virtual ~nsNullPrincipal() {}
 
   bool SubsumesInternal(nsIPrincipal* aOther, DocumentDomainConsideration aConsideration) override
   {
--- a/caps/nsNullPrincipalURI.cpp
+++ b/caps/nsNullPrincipalURI.cpp
@@ -91,17 +91,18 @@ nsNullPrincipalURI::GetAsciiHostPort(nsA
 {
   return NS_ERROR_NOT_IMPLEMENTED;
 }
 
 NS_IMETHODIMP
 nsNullPrincipalURI::GetAsciiSpec(nsACString &_spec)
 {
   nsAutoCString buffer;
-  (void)GetSpec(buffer);
+  // Ignore the return value -- nsNullPrincipalURI::GetSpec() is infallible.
+  Unused << GetSpec(buffer);
   NS_EscapeURL(buffer, esc_OnlyNonASCII | esc_AlwaysCopy, _spec);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsNullPrincipalURI::GetHost(nsACString &_host)
 {
   _host.Truncate();
--- a/caps/nsPrincipal.cpp
+++ b/caps/nsPrincipal.cpp
@@ -94,20 +94,20 @@ nsPrincipal::Init(nsIURI *aCodebase, con
 
   mCodebase = NS_TryToMakeImmutable(aCodebase);
   mCodebaseImmutable = URIIsImmutable(mCodebase);
   mOriginAttributes = aOriginAttributes;
 
   return NS_OK;
 }
 
-void
+nsresult
 nsPrincipal::GetScriptLocation(nsACString &aStr)
 {
-  mCodebase->GetSpec(aStr);
+  return mCodebase->GetSpec(aStr);
 }
 
 /* static */ nsresult
 nsPrincipal::GetOriginForURI(nsIURI* aURI, nsACString& aOrigin)
 {
   if (!aURI) {
     return NS_ERROR_FAILURE;
   }
@@ -801,32 +801,34 @@ bool
 nsExpandedPrincipal::IsOnCSSUnprefixingWhitelist()
 {
   // CSS Unprefixing Whitelist is a per-origin thing; doesn't really make sense
   // for an expanded principal. (And probably shouldn't be needed.)
   return false;
 }
 
 
-void
+nsresult
 nsExpandedPrincipal::GetScriptLocation(nsACString& aStr)
 {
   aStr.Assign("[Expanded Principal [");
   for (size_t i = 0; i < mPrincipals.Length(); ++i) {
     if (i != 0) {
       aStr.AppendLiteral(", ");
     }
 
     nsAutoCString spec;
-    nsJSPrincipals::get(mPrincipals.ElementAt(i))->GetScriptLocation(spec);
+    nsresult rv =
+      nsJSPrincipals::get(mPrincipals.ElementAt(i))->GetScriptLocation(spec);
+    NS_ENSURE_SUCCESS(rv, rv);
 
     aStr.Append(spec);
-
   }
   aStr.Append("]]");
+  return NS_OK;
 }
 
 //////////////////////////////////////////
 // Methods implementing nsISerializable //
 //////////////////////////////////////////
 
 NS_IMETHODIMP
 nsExpandedPrincipal::Read(nsIObjectInputStream* aStream)
--- a/caps/nsPrincipal.h
+++ b/caps/nsPrincipal.h
@@ -29,17 +29,17 @@ public:
   bool IsCodebasePrincipal() const override { return true; }
   nsresult GetOriginInternal(nsACString& aOrigin) override;
 
   nsPrincipal();
 
   // Init() must be called before the principal is in a usable state.
   nsresult Init(nsIURI* aCodebase, const mozilla::PrincipalOriginAttributes& aOriginAttributes);
 
-  virtual void GetScriptLocation(nsACString& aStr) override;
+  virtual nsresult GetScriptLocation(nsACString& aStr) override;
   void SetURI(nsIURI* aURI);
 
   /**
    * Computes the puny-encoded origin of aURI.
    */
   static nsresult GetOriginForURI(nsIURI* aURI, nsACString& aOrigin);
 
   /**
@@ -76,17 +76,17 @@ public:
   NS_IMETHOD QueryInterface(REFNSIID aIID, void** aInstancePtr) override;
   NS_IMETHOD GetHashValue(uint32_t* aHashValue) override;
   NS_IMETHOD GetURI(nsIURI** aURI) override;
   NS_IMETHOD GetDomain(nsIURI** aDomain) override;
   NS_IMETHOD SetDomain(nsIURI* aDomain) override;
   NS_IMETHOD GetBaseDomain(nsACString& aBaseDomain) override;
   virtual bool AddonHasPermission(const nsAString& aPerm) override;
   virtual bool IsOnCSSUnprefixingWhitelist() override;
-  virtual void GetScriptLocation(nsACString &aStr) override;
+  virtual nsresult GetScriptLocation(nsACString &aStr) override;
   nsresult GetOriginInternal(nsACString& aOrigin) override;
 
   PrincipalKind Kind() override { return eExpandedPrincipal; }
 
 protected:
   virtual ~nsExpandedPrincipal();
 
   bool SubsumesInternal(nsIPrincipal* aOther, DocumentDomainConsideration aConsideration) override;
--- a/caps/nsSystemPrincipal.cpp
+++ b/caps/nsSystemPrincipal.cpp
@@ -26,20 +26,21 @@ NS_IMPL_QUERY_INTERFACE_CI(nsSystemPrinc
                            nsIPrincipal,
                            nsISerializable)
 NS_IMPL_CI_INTERFACE_GETTER(nsSystemPrincipal,
                             nsIPrincipal,
                             nsISerializable)
 
 #define SYSTEM_PRINCIPAL_SPEC "[System Principal]"
 
-void
+nsresult
 nsSystemPrincipal::GetScriptLocation(nsACString &aStr)
 {
     aStr.AssignLiteral(SYSTEM_PRINCIPAL_SPEC);
+    return NS_OK;
 }
 
 ///////////////////////////////////////
 // Methods implementing nsIPrincipal //
 ///////////////////////////////////////
 
 NS_IMETHODIMP
 nsSystemPrincipal::GetHashValue(uint32_t *result)
--- a/caps/nsSystemPrincipal.h
+++ b/caps/nsSystemPrincipal.h
@@ -33,17 +33,17 @@ public:
   NS_IMETHOD EnsureCSP(nsIDOMDocument* aDocument, nsIContentSecurityPolicy** aCSP) override;
   NS_IMETHOD GetPreloadCsp(nsIContentSecurityPolicy** aPreloadCSP) override;
   NS_IMETHOD EnsurePreloadCSP(nsIDOMDocument* aDocument, nsIContentSecurityPolicy** aCSP) override;
   NS_IMETHOD GetBaseDomain(nsACString& aBaseDomain) override;
   nsresult GetOriginInternal(nsACString& aOrigin) override;
 
   nsSystemPrincipal() {}
 
-  virtual void GetScriptLocation(nsACString &aStr) override;
+  virtual nsresult GetScriptLocation(nsACString &aStr) override;
 
 protected:
   virtual ~nsSystemPrincipal(void) {}
 
   bool SubsumesInternal(nsIPrincipal *aOther, DocumentDomainConsideration aConsideration) override
   {
     return true;
   }
--- a/js/xpconnect/loader/mozJSSubScriptLoader.cpp
+++ b/js/xpconnect/loader/mozJSSubScriptLoader.cpp
@@ -362,24 +362,24 @@ AsyncScriptLoader::OnStreamComplete(nsII
 
     if (aLength > INT32_MAX) {
         return ReportError(cx, LOAD_ERROR_CONTENTTOOBIG, uri);
     }
 
     RootedFunction function(cx);
     RootedScript script(cx);
     nsAutoCString spec;
-    uri->GetSpec(spec);
+    nsresult rv = uri->GetSpec(spec);
+    NS_ENSURE_SUCCESS(rv, rv);
 
     RootedObject target_obj(cx, mTargetObj);
 
-    nsresult rv = PrepareScript(uri, cx, target_obj, spec.get(),
-                                mCharset,
-                                reinterpret_cast<const char*>(aBuf), aLength,
-                                mReuseGlobal, &script, &function);
+    rv = PrepareScript(uri, cx, target_obj, spec.get(), mCharset,
+                       reinterpret_cast<const char*>(aBuf), aLength,
+                       mReuseGlobal, &script, &function);
     if (NS_FAILED(rv)) {
         return rv;
     }
 
     JS::Rooted<JS::Value> retval(cx);
     rv = EvalScript(cx, target_obj, &retval, uri, mCache, script, function);
 
     if (NS_SUCCEEDED(rv)) {
--- a/js/xpconnect/src/Sandbox.cpp
+++ b/js/xpconnect/src/Sandbox.cpp
@@ -1759,17 +1759,18 @@ xpc::EvalInSandbox(JSContext* cx, Handle
     nsCOMPtr<nsIPrincipal> prin = sop->GetPrincipal();
     NS_ENSURE_TRUE(prin, NS_ERROR_FAILURE);
 
     nsAutoCString filenameBuf;
     if (!filename.IsVoid() && filename.Length() != 0) {
         filenameBuf.Assign(filename);
     } else {
         // Default to the spec of the principal.
-        nsJSPrincipals::get(prin)->GetScriptLocation(filenameBuf);
+        nsresult rv = nsJSPrincipals::get(prin)->GetScriptLocation(filenameBuf);
+        NS_ENSURE_SUCCESS(rv, rv);
         lineNo = 1;
     }
 
     // We create a separate cx to do the sandbox evaluation. Scope it.
     RootedValue v(cx, UndefinedValue());
     RootedValue exn(cx, UndefinedValue());
     bool ok = true;
     {
--- a/js/xpconnect/src/XPCJSRuntime.cpp
+++ b/js/xpconnect/src/XPCJSRuntime.cpp
@@ -1663,17 +1663,20 @@ GetCompartmentName(JSCompartment* c, nsC
                    bool replaceSlashes)
 {
     if (js::IsAtomsCompartment(c)) {
         name.AssignLiteral("atoms");
     } else if (*anonymizeID && !js::IsSystemCompartment(c)) {
         name.AppendPrintf("<anonymized-%d>", *anonymizeID);
         *anonymizeID += 1;
     } else if (JSPrincipals* principals = JS_GetCompartmentPrincipals(c)) {
-        nsJSPrincipals::get(principals)->GetScriptLocation(name);
+        nsresult rv = nsJSPrincipals::get(principals)->GetScriptLocation(name);
+        if (NS_FAILED(rv)) {
+            name.AssignLiteral("(unknown)");
+        }
 
         // If the compartment's location (name) differs from the principal's
         // script location, append the compartment's location to allow
         // differentiation of multiple compartments owned by the same principal
         // (e.g. components owned by the system or null principal).
         CompartmentPrivate* compartmentPrivate = CompartmentPrivate::Get(c);
         if (compartmentPrivate) {
             const nsACString& location = compartmentPrivate->GetLocation();