437014 - nsIContentPolicy::shouldLoad no longer called for favicons

Status: RESOLVED WORKSFORME

(Firefox :: General, defect)

Description

[Mike Perry]

It appears that the new places in FF3.0 code no longer calls nsIContentPolicy::shouldLoad for favicons. In the past, shouldLoad was called with a requestOrigin url of the browser overlay. Now it seems to be not called at all for favicon loads.

Comment 1

[Gervase Markham [:gerv]]

Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv

Comment 2

[Daniel Veditz [:dveditz]]

I'm sorry I missed this one. I don't seriously expect it to "block" Gecko 2.0, but it should be fixed anyway.

Comment 4

[Wladimir Palant]

Right, I meant to file a bug on this but forgot. It only affects the legacy /favicon.ico request, favicons specified in the <link> tag go through content policies as expected.

Comment 5

[Mike Beltzner [:beltzner, not reading bugmail]]

As per your expectations!

Comment 6

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

The mixed content blocker needs to be aware of favicons linked from <link rel=icon> in order to correctly block <link rel=icon href=http://> in its strictest setting.

Comment 7

[Marco Bonardo [:mak]]

I don't think this is actually a bug in favicons code, based on comment 4 looks like the browser code just fallbacks to trying the default icon if there is no <link ref=icon> specified in the page.
This code seems to confirm that http://mxr.mozilla.org/mozilla-central/source/browser/base/content/tabbrowser.xml#557
Though, getLinkIconURI always goes through ContentPolicy
http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#3260

Should we also check the content policy for the default icon fallback, even if we make up an uri using docURIObject.prePath, that afaict preserves the current documentURI schema?

Comment 8

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

(In reply to Marco Bonardo [:mak] from comment #7)
> Should we also check the content policy for the default icon fallback, even
> if we make up an uri using docURIObject.prePath, that afaict preserves the
> current documentURI schema?

It isn't necessary to check the default icon fallback for the mixed content blocker because it will always be same-origin. And, I think that nsIContentPolicy should not be applied to requests that aren't based on web content such as OCSP requests and the default <scheme>://<hostname>:<port>/favicon.ico request.

Comment 9

[Tom Ritter [:tjr]]

This seems to be a question of whether ContentPolicy should be applied to default favicon requests, which seems like its in your wheelhouse...

Comment 10

[Christoph Kerschbaumer [:ckerschb]]

I just verified that we perform Content Security checks (formerly known as shouldLoad) for Favicons. Loads happen with type: 'TYPE_INTERNAL_IMAGE_FAVICON' here:
https://searchfox.org/mozilla-central/rev/557a0e222dd104c5d805ba344c45d6abc27d3db0/dom/security/nsContentSecurityManager.cpp#862

I don't know what Bug fixed that exactly, but must have been one of those dependencies: Bug 1143922.