1099382 - GetCanonicalClone() fails on nsIURI implementations that don't support SetUserPass()

Status: RESOLVED INCOMPLETE

(Core :: DOM: Security, defect)

Description

[David Finch]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:33.0) Gecko/20100101 Firefox/33.0
Build ID: 20141106120505

Steps to reproduce:

It's complicated. The newest Distill/AlertBox addon added a "static" mode, but it was being broken by the presence of script blocking plugins such as YesScript, which use nsScriptSecurityManager's DomainPolicy blacklist. Simply having the blacklist enabled would break AlertBox's checking, even with nothing specifically blacklisted.

Steps to reproduce, for the sake of completeness: Create a watch in Distill/AlertBox. Edit the watch's json config to set dynamic="false". Check the watch. Should succeed. Install or enable YesScript. Check the watch again. Should time out after 60 seconds.



Actual results:

Distill's check silently fails to run, and eventually the addon's UI reports a timeout.

My theory on what exactly happened, based on Firefox's code:
Firefox's nsXBLDocumentInfo constructor, on non-chrome:// urls, calls nsScriptSecurityManager::PolicyAllowsScript() to determine if scripts should be allowed to run.
PolicyAllowsScript(), if domain policy is enabled, calls DomainSet::Contains() and DomainSet::ContainsSuperDomain() against the blacklist.
Contains() and ContainsSuperDomain() both call GetCanonicalClone() to cleanup the url (strip path and password).
GetCanonicalClone() calls SetUserPass() on the url to set it to an empty string.

If SetUserPass() reports a failure, GetCanonicalClone() fails, DomainSet::Contains() fails, and nsScriptSecurityManager::PolicyAllowsScript() fails, then nsXBLDocumentInfo's constructor denies script access in case of failure.

SetUserPass() is only implemented for nsStandardURL, and all other nsIURI implementations report failure as they don't support it.

I think Distill's "static check" mode uses a hidden iframe with a resource:// path, which is one of those URI protocols that reports failure in SetUserPass(). This causes it to be incorrectly blocked, but only when a domain policy blacklist exists, regardless of the blacklist's contents.


Expected results:

Distill's static check should have succeeded regardless of whether or not domain policy was active.

I think GetCanonicalClone() ought to permit NS_ERROR_NOT_IMPLEMENTED results from nsIURI::SetUserPass. Then nsJARURI::SetUserPass() and nsSimpleURI::SetUserPass() should return NS_ERROR_NOT_IMPLEMENTED (as nsNullPrincipalURI::SetUserPass already does) instead of NS_ERROR_FAILURE, to distinguish their empty implementations from more serious errors.

Comment 1

[Christoph Kerschbaumer [:ckerschb]]

David, I don't know how it's possible that this bug felt through the cracks completely. Currently I am triaging all the bugs within DOM:Security and came across this one. Is it still an issue?

Comment 2

[Christoph Kerschbaumer [:ckerschb]]

No response, hence I am closing as INCOMPLETE for now.