550290 - nsSidebar.js check for icon validity flawed

Status: RESOLVED FIXED

(Firefox :: Search, defect)

Description

[Mycroft Project (CC)]

@ nsSidebar.js#144

144     // Make sure we're using HTTP, HTTPS, or FTP and refering to a
145     // .gif/.jpg/.jpeg/.png/.ico file for the icon.
146     if (iconURL &&
147         ! /^(https?|ftp):\/\/.+\.(gif|jpg|jpeg|png|ico)$/i.test(iconURL))
148       throw "Unsupported search icon URL.";
149   }

fails for favicon of form http://example.com/favicon.ico?123numberhere

this also breaks search engine installs with addSearchProvider where the image in .xml description is ok in terms of the above but the page favicon for the fallback is not.

Comment 1

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

Attachment: loosen the restriction

I think these checks are all pretty dumb, but the lame attempt at file extension checking is the dumbest, since it can easily be circumvented and doesn't really protect against anything (worst case we will load and store bogus data as the icon).

Comment 2

[Ryan Flint [:rflint] (ping via IRC for reviews)]

Comment on attachment 434353 [details] [diff] [review]
loosen the restriction

Certainly better than what we had!

If we want to drop these checks entirely, maybe we could just pass these through the script security manager with a null principal? Though I'm not totally sure that'd give us any benefit over these regexps...

Comment 3

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

https://hg.mozilla.org/mozilla-central/rev/87b28ba15a8f