732413 - DISALLOW_INHERIT_PRINCIPAL is ignored when calling checkLoadURIWithPrincipal with aPrincipal=system principal ("javascript:" links can be set as the home page when dragged from chrome)

Status: RESOLVED FIXED

(Core :: Security, defect)

Description

[Ioana (away)]

Mozilla/5.0 (Windows NT 6.1; rv:11.0) Gecko/20100101 Firefox/11.0
Mozilla/5.0 (Windows NT 5.1; rv:11.0) Gecko/20100101 Firefox/11.0
Mozilla/5.0 (X11; Linux x86_64; rv:11.0) Gecko/20100101 Firefox/11.0

STR1:
1. Enter "javascript:1" in the search bar.
2. Select the text from the search bar and drag it to the Home button.

STR2:
1. Enter "javascript:1" in the location bar.
2. Select the text from the search bar and drag it to the Home button.

STR3:
1. Select "javascript:1" from this comment and drag it to the Bookmarks button.
2. Click on the Bookmarks button.
3. Select the "javascript:1" bookmark and drag it to the Home button.

Expected Results:
The user cannot set "javascript:1" (or any other "javascript:...") as his Home page by dropping it onto the Home button.

Actual Results:
The confirmation pop-up asking whether the user wants to set "javascript:1" as his Home page or not is displayed. The user can set it without any issues.

Comment 1

[:Gavin Sharp [email: [e-mail]]]

This happens because DISALLOW_INHERIT_PRINCIPAL is ignored when checkLoadURI()'s source principal is the system principal (there's an early return in nsScriptSecurityManager::CheckLoadURIWithPrincipal if aPrincipal == mSystemPrincipal).

bz, is there any chance we could change that?

Comment 2

[Boris Zbarsky [:bzbarsky]]

I think so, yes.  Specifically, hoisting the DISALLOW_INHERIT_PRINCIPAL check above the mSystemPrincipal check would make a lot of sense to me.  I would have thought we had an existing bug on that, but I don't see one....

Comment 3

[:Gavin Sharp [email: [e-mail]]]

Attachment: patch

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 605884 [details] [diff] [review]
patch

It's worth adding to the end of that comment that we want to do this even for the system principal.

r=me

Comment 5

[:Gavin Sharp [email: [e-mail]]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bf542c70745f

Comment 6

[Mounir Lamouri (:mounir)]

https://hg.mozilla.org/mozilla-central/rev/bf542c70745f