871262 - Fix rooting hazards in the CAPS code

Status: RESOLVED FIXED

(Core :: Security: CAPS, defect)

Description

[(no longer active)]

Attachment: Patch (v1)

Comment 1

[Till Schneidereit [:till]]

Comment on attachment 748529 [details] [diff] [review]
Patch (v1)

Review of attachment 748529 [details] [diff] [review]:
-----------------------------------------------------------------

Nice! Feel free to ignore my nits, if you'd rather not touch this again.

::: caps/src/nsScriptSecurityManager.cpp
@@ +634,5 @@
>      if (NS_FAILED(rv))
>          return rv;
>  
>      if (securityLevel.level == SCRIPT_SECURITY_UNDEFINED_ACCESS)
>      {   

whitespace

@@ +661,5 @@
>  
>          case SCRIPT_SECURITY_SAME_ORIGIN_ACCESS:
>              {
>                  nsCOMPtr<nsIPrincipal> principalHolder;
> +                if(jsObject)

if (

@@ +1758,1 @@
>                        nsIXPCSecurityManager::ACCESS_GET_PROPERTY, 

whitespace

@@ +2051,5 @@
>  
>  #ifdef DEBUG
>  // static
>  nsIPrincipal*
> +nsScriptSecurityManager::old_doGetObjectPrincipal(JSObject *aObj_,

Any reason not to leave this as `aObj` and use `obj` below?

(If the answer is "yes: lots of changes would be required", I would totally understand.)

@@ +2268,5 @@
>  }
>  
>  nsresult
>  nsScriptSecurityManager::CheckXPCPermissions(JSContext* cx,
> +                                             nsISupports* aObj, JSObject* aJSObject_,

Same here: I'd prefer keeping `aJSObject` and introducing `jsObject`, as you did elsewhere.

Comment 2

[(no longer active)]

Comment on attachment 748529 [details] [diff] [review]
Patch (v1)

Review of attachment 748529 [details] [diff] [review]:
-----------------------------------------------------------------

::: caps/src/nsScriptSecurityManager.cpp
@@ +2051,5 @@
>  
>  #ifdef DEBUG
>  // static
>  nsIPrincipal*
> +nsScriptSecurityManager::old_doGetObjectPrincipal(JSObject *aObj_,

The reason is laziness!  But I'll make this change as you requested.

Comment 3

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/71e04d6924bf

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/71e04d6924bf