404627 - [FIX]XPinstall whitelist bypass using refresh after fix for bug 402649

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Daniel Veditz [:dveditz]]

bug 402649 described a way to fake a referer header, which amongst the various CSRF attacks envisioned by the reporter also allowed a trivial bypass of the xpinstall site blocking code -- you simply specify https://addons.mozilla.org as the referer.

The fix made the referer always blank, which xpinstall then interprets as having been entered into the address bar by the user (or as a local file URI). When I try the refresh trick without the race condition the xpinstall is correctly blocked even though no referer is actually sent over the wire. The blocking actually uses an internal referer property set on the channel that may not always match the physical referer, we need to get that set in cases like bug 402649

Comment 1

[Daniel Veditz [:dveditz]]

This can be tested with the testcase from bug 402649, attachment 287538 [details]

If the referring site is http://bugzilla.mozilla.org/ then the addon is correctly blocked (this is because of the principal-comparing), anything else and it's allowed. Prior to the fix the referer had to be https://addons.mozilla.org to bypass the whitelist.

Handy harmless xpi link for testing: http://releases.mozilla.org/pub/mozilla.org/addons/3867/together_with_foxkeh-0.1.6.1-firefox.xpi

Comment 2

[Daniel Veditz [:dveditz]]

I hadn't checked trunk and made a bad assumption that the patches were equivalent. Trunk does not exhibit this bug, 1.8-branch only

Comment 3

[Boris Zbarsky [:bzbarsky]]

Sounds like we should take the trunk fix for bug 402649 on the branch, then.  That's the only way we can get a sane referrer here.

jst, does that sound OK to you?

We should create an automated testcase for this, if we can....

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: Like so

Comment 5

[Johnny Stenback (:jst)]

Comment on attachment 289554 [details] [diff] [review]
Like so

Yes, seems like this is the way to go.

Comment 6

[Boris Zbarsky [:bzbarsky]]

It's possible that this causes bug 405643...

Comment 7

[Daniel Veditz [:dveditz]]

(In reply to comment #6)
> It's possible that this causes bug 405643...

No, because this is not fixed on the branch yet :-)

bug 402649 was in the regression range, but we've now narrowed it down to the
jar: xss fix.

Comment 8

[Boris Zbarsky [:bzbarsky]]

I meant that it was possible that this patch not being landed was causing bug 405643.

But yes, good to know it's the jar: stuff.

Comment 9

[Daniel Veditz [:dveditz]]

Comment on attachment 289554 [details] [diff] [review]
Like so

approved for 1.8.1.12, a=dveditz for release-drivers

Comment 10

[Boris Zbarsky [:bzbarsky]]

Fixed on branch.

Comment 11

[Samuel Sidler (old account; do not CC)]

Verified fixed using the steps in comment 1. Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.12) Gecko/2008012820 Firefox/2.0.0.12

Comment 12

[Alexander Sack]

distro patches block 1.8.0.15

Comment 13

[Alexander Sack]

Comment on attachment 289554 [details] [diff] [review]
Like so

unmodified distro patch; caillon, please sign off for landing.

Comment 14

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 289554 [details] [diff] [review]
Like so

a=caillon for 1.8.0.15 and I already committed this as part of the commit for bug 402649