1179241 - Using cookie bomb trick it is possible to get content of the attachment of any security bug.

Status: RESOLVED WONTFIX

(Bugzilla :: Attachments & Requests, defect)

Description

[Mario Gomes]

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/43.0.2357.130 Safari/537.36

Steps to reproduce:

Hi,

I have found a quite interesting security bug on bugzilla

since attachments are hosted in a subdomain, it is possible to cookie bomb any page at bugzilla.

This kind vulnerability was first found by: Egor Homakov (Have a look at: http://homakov.blogspot.com.br/2014/01/cookie-bomb-or-lets-break-internet.html ) 


that said, it is possible cookie bomb a bugzilla attachment domain and then get the attachment url containning the "t"(security token) and then getting the content of the attachment.

when you visit the attachment of a security bug bugzilla will:
1. visit https://bugzilla.mozilla.org/attachment.cgi?id=[ATTACHMENT-ID]
2. redirect to https://bug[BUG-ID].bugzilla.mozilla.org/attachment.cgi?id=[ATTACHMENT-ID]&t=[security-token]

once the cookie bomb the attachment domain, it'll present a error page which will make the "t" still valid to be seen by a browser that doesn't have it bombed.

it's complicated for explainning so let's exploit it :)

i'm only able to exploit it using firefox.

steps to reproduce(this works only in Mozilla FIREFOX):
1. Log in https://bugzilla-dev.allizom.org
2. visit bugzilla-dev.allizom.org/attachment.cgi?id=8591427
3. in the prompted box insert the ID of the security bug you'd like to get the token.
4. wait 3 seconds.
5. now drag and drop the content of the image to the box below it.
6. now copy the url displayed in the box and open it in another browser that you're not logged in bugzilla.
7. see the content of the security bug.

I think this is critical, as this isn't hard to make a user drag and drop the content of a image to a box and bugzilla
attachments are not protected against clickjacking, it can be framed and exploited from www.evilpage.com.

let me know if you understand this vulnerability.

Cheers,

Comment 1

[:glob ✱]

this is a deployment issue not a product issue, and is likely either a duplicate of bug 38862 (which allows bugzilla sites to use a different domain for attachments), or bug 1094835 (which is for moving bmo's attachments to a separate domain instead of a subdomain).  (ie. it's in exactly the same situation as bug 1177632).

Comment 2

[:glob ✱]

duplicating this against bmo's attachment domain deployment bug (but leaving sec-bounty? set).

Comment 3

[Gervase Markham [:gerv]]

I don't think that switching to a non-subdomain attachment domain will help here.

This is how I think this works:

* The attachment load tokens are one-shot. (To see this, shift-reload an attachment - you get a new one.) 
* Normally, setting the src of an <img> to a known attachment URL in a logged-in user's browser will redirect to a tokenized attachment URL, but then it'll load the data, so the token is used and useless.
* If you can cookie-bomb the attachment domain in a particular browser, the redirect still works, but the load never happens, and so the token remains usable.
* The attacker can then steal the token using image drag-drop, which puts the src of the image into the text box. Mario used an SVG background-image to show some instructional text even though that "actual" image (the attachment page) didn't load and wouldn't display if it did.

So it seems to me that even if the attachment domain were bmoattachments.org, other attachments would still be hosted at that domain and so could be cookie-bombed. 

Possible fixes:

1) Switch to bmoattachments.org and add it to the Public Suffix List so cookies may not be set at that level 
   (githubusercontent.com is already on the list; this would be a similar idea)
2) Get the webserver to keep processing (i.e. by ignoring all cookies) if the Cookie header is too long
3) Make the redirect _also_ fail if the Cookie header is too long
4) Try and refuse to load attachment pages when the load is not at the top level (not sure if this is reliably detectable)

Gerv

Comment 4

[Mario Gomes]

(In reply to Gervase Markham [:gerv] from comment #3)
> So it seems to me that even if the attachment domain were
> bmoattachments.org, other attachments would still be hosted at that domain
> and so could be cookie-bombed. 

Exactly, that's what I was about to tell him it doesn't matter the domain as long as I can still cookie bomb it
this vulnerability still present. :)
 
> Possible fixes:
> 1) Switch to bmoattachments.org and add it to the Public Suffix List so
> cookies may not be set at that level 
>    (githubusercontent.com is already on the list; this would be a similar
> idea)
This one would definitely work! :D
> 2) Get the webserver to keep processing (i.e. by ignoring all cookies) if
> the Cookie header is too long
Yea, it would work. But it'd leave the server more vulnerable to DoS attacks.
> 3) Make the redirect _also_ fail if the Cookie header is too long
Wouldn't work! As you might have seen on exploit, I first wait the page load https://bugzilla.mozilla.org/attachment.cgi?id=[ATTACHMENT-ID]
then while it returns the redirect code I cookie bomb the attachment domain in order to getting the token(as If I did it before the https://bugzilla.mozilla.org/attachment.cgi?id=[ATTACHMENT-ID] is load, it wouldn't work once it'd also cookie bomb the bugzilla mainly domain too).

> 4) Try and refuse to load attachment pages when the load is not at the top
> level (not sure if this is reliably detectable)
I'm not aware of way of doing that but if there's that's definitely a good idea.

Comment 5

[Mario Gomes]

Btw, what would be the appropriate "sec" and "ws-sec" for this bug ?

Comment 6

[Mario Gomes]

I've worked on a new PoC that reduces the need of user interaction.

Reproduce:
1. once you're logged in, go to https://bugzilla-dev.allizom.org/attachment.cgi?id=8591429 
2. put the ID of the bug you intend to obtain the URL.
3. Now drag the image in the page to somewhere.
4. See the url of the attachment with its token, copy it and open it on the different browser which it is not logged in bugzilla.
5. see the attachment.

Comment 7

[Mario Gomes]

Hi,

There's a easy and alternative way for fixing this vulnerability, the token should be passed as a cookie(protected with HttpOnly) instead of pass it through URL param then there wouldn't be a way for getting the cookie.

Comment 8

[Frédéric Buclin]

The alternate attachment host generates and gets no cookies at all. That's the reason why we pass the token in the URL.

(In reply to Mario Gomes from comment #0)
> 7. see the content of the security bug.

I don't see how you could see the content of a security bug using your trick. This has nothing to do with tokens.

Comment 9

[Mario Gomes]

(In reply to Frédéric Buclin from comment #8)
> I don't see how you could see the content of a security bug using your
> trick. This has nothing to do with tokens.

Yep, sorry about that misspelled it. I meant to say "the content of the attachment" :)

Comment 10

[Gervase Markham [:gerv]]

(In reply to Mario Gomes from comment #7)
> Hi,
> 
> There's a easy and alternative way for fixing this vulnerability, the token
> should be passed as a cookie(protected with HttpOnly) instead of pass it
> through URL param then there wouldn't be a way for getting the cookie.

The entire point of the separate domains is that they don't share cookies - so the attachments don't have access to Bugzilla auth cookies. So this won't work.

Gerv

Comment 11

[:glob ✱]

(In reply to Gervase Markham [:gerv] from comment #3)
> 1) Switch to bmoattachments.org and add it to the Public Suffix List so
> cookies may not be set at that level 

https://github.com/publicsuffix/list/pull/10

Comment 12

[Gervase Markham [:gerv]]

glob: We can fix it that way, but that means that everyone who wants to run a secure Bugzilla has to register a domain and get it added to the PSL. That's not awesome. Is there any way to implement one of my other fixes?

If not, could we register bzattachments.org, add *.bzattachments.org to the PSL, and then offer a DNS delegation to anyone who wants to run a secure Bugzilla? (Perhaps there's some group of DNS-loving geeks out there who could do this as a public service.) So BMO would used bmo.bzattachments.org (with bugs at bug123456.bmo.bzattachments.org), etc. 

Gerv

Comment 13

[:glob ✱]

(In reply to Gervase Markham [:gerv] from comment #12)
> glob: We can fix it that way

yup .. for bmo that's the approach i want to take as it guards us against other potential attacks.

> Is there any way to implement one of my other fixes?

#4 sounds the most viable .. i suspect setting X-Frame-Options to 'deny' would do it, but it would break the attachment details page which shows the content within a frame.

Comment 14

[Mario Gomes]

(In reply to Gervase Markham [:gerv] from comment #10)
> The entire point of the separate domains is that they don't share cookies -
> so the attachments  don't have access to Bugzilla auth cookies. So this won't
> work.
> 
> Gerv

So... why don't you send the "t" param through a POST request? It'd work quite fine, since i'm only able to get the token 'cause it is in page's URL. Passing param through POST would hide it and even if I could cookie bomb the domain there wouldn't be a way for getting the token.

Comment 15

[Mario Gomes]

Attachment: New PoC to work with new Bugzilla Attachment Domain.

Comment 16

[Mario Gomes]

Attachment: New PoC to work with new Bugzilla Attachment Domain

Comment 17

[Mario Gomes]

Attachment: New PoC to work with new Bugzilla Attachment Domain

Sorry about the spamming.

Just finished the last PoC which works with the new bugzilla attachment domain.

Comment 18

[Gervase Markham [:gerv]]

Mario: I can't get the new PoC to work. Can you give exact steps to reproduce with the new PoC?

Gerv

Comment 19

[Mario Gomes]

Attachment: New PoC to work with new Bugzilla Attachment Domain

Comment 20

[Mario Gomes]

> Mario: I can't get the new PoC to work. Can you give exact steps to
> reproduce with the new PoC?
> 
> Gerv

I think you did not give enough time for the <img> tag finishes loading the attachment's url and get the token. I've worked in a new PoC which will make sure the <img> finishes loading.

Also, this PoC is meant to work on bugzilla.mozilla.org so when requested put the ID of a BMO attachment.

Steps to reproduce(works only on Firefox):
1. Log in bugzilla.
2. now go to https://bugzilla.mozilla.org/attachment.cgi?id=8640518
3. in the prompted text box, put the ID of the attachment you wanna get the URL.
4. wait until the image appears.
5. now drag the image to somewhere.
6. now the url of the attachment(with its valid token) will be prompted, copy it and open it in the unloged browser.
7. see the content of the attachment.

Comment 21

[Gervase Markham [:gerv]]

Yep, confirmed. I suspected that changing domain wouldn't work.

Gerv

Comment 22

[:glob ✱]

(In reply to Gervase Markham [:gerv] from comment #21)
> Yep, confirmed. I suspected that changing domain wouldn't work.

that's expected -- we're waiting on the PSL to be updated.

Comment 23

[Gervase Markham [:gerv]]

Well OK, but just updating the PSL won't fix the issue - the update has to be rolled out to every user agent, which can take a very long time, particularly for Apple products.

Are there not any other mitigations we can put in place? E.g. can we do what Mario suggests, and instead of serving a 301 redirect from attachment.cgi, serve a page which has a small script which does a POST request to bmoattachments.org with the auth code as a parameter? We could also add a meta redirect with a 3-second timeout as a fallback for if script was disabled, but in the common case, people would be protected (by enabling JS, ironically).

Gerv

Comment 24

[Frédéric Buclin]

(In reply to Gervase Markham [:gerv] from comment #23)
> Well OK, but just updating the PSL won't fix the issue

This bug is about Bugzilla, not bmo.

Comment 25

[Gervase Markham [:gerv]]

(In reply to Frédéric Buclin from comment #24)
> (In reply to Gervase Markham [:gerv] from comment #23)
> > Well OK, but just updating the PSL won't fix the issue
> 
> This bug is about Bugzilla, not bmo.

That too. I've sent out an enquiry about whether we can find some kind DNS geeks willing to manage subdomain registrations for something like *.bzattachments.org which could be added to the PSL, so everyone with a Bugzilla could get a subdomain for their attachments (as noted in comment 12). But that fix would be some way off anyway.

glob/LpSolit: what about my suggestion in comment 23?

Or could we tie tokens to an IP address? Given that they are one-shot, and intended to be used a fraction of a second after being issued, there's not much chance of someone's IP address changing between issuance and use. And that would significantly reduce the universe of possible attackers.

I also filed bug 1189271 to get Firefox to ponder mitigations for cookie bombing.

Gerv

Comment 26

[Frédéric Buclin]

We can tie tokens to their IP address; we already do that for logincookies, or we can use POST to pass the token. The latter is not hard to do.

Comment 27

[Frédéric Buclin]

Hum, no, it's not that trivial to use POST, because we use action="view" for both direct view of attachments, and their view in the iframe in the "attachment details" page. But the iframe is in a sandbox, meaning that JS and form submission are disabled and so the attachment is never displayed.

Comment 28

[Mario Gomes]

Good point. Why dont put the token directly in the "attachment details" page? Instead of using redirect, the iframe should be pointing directly to the attachment domain with its token.

Comment 29

[Frédéric Buclin]

That's fine, I found a workaround. :)

Comment 30

[Frédéric Buclin]

Attachment: patch for trunk, v1

Use POST instead of GET to pass the token to the alternate host, unless a valid hash token is present in the URL. The hash token is only generated in the "Attachment Details" page, and so is unaccessible to the attachment in the iframe, as the iframe is in a sandbox and so blocks all scripts.

Mario/Gerv: I will let you play with this patch as I didn't try to bomb my own installation.

Comment 31

[:glob ✱]

(In reply to Gervase Markham [:gerv] from comment #23)
> Well OK, but just updating the PSL won't fix the issue.

my response was based on your comment 3, where you stated it was a fix for this issue.
no matter, it's good to have for bmo regardless :)

(In reply to Frédéric Buclin from comment #30)
> Use POST instead of GET to pass the token to the alternate host, unless a
> valid hash token is present in the URL. The hash token is only generated in
> the "Attachment Details" page, and so is unaccessible to the attachment in
> the iframe, as the iframe is in a sandbox and so blocks all scripts.

this change adds an intermediary html page when requesting attachments from non-public bugs.  this will break anything which integrates with bugzilla expecting a request to attachment.cgi?id=$attach_id to return the attachment payload.

i've seen this used on bmo where a file's dependencies are attached to a bug, and are then referenced by a html file attached after them (pretty sure i've seen security POCs attached in this way).

scripts should use the api to fetch attachment content, so i'm not overly concerned about breaking them.
for the multi-part attachment use-case, i'm not sure if that's something we should continue to support -- multi-part POCs could be attached as an archive instead.

i'm not saying this patch is a bad, just that we need to be aware of its impact and reflect that impact in the release notes.


fwiw, the patch looks sane to me aside from the redirect page.

phrases like "click this button" should be avoided - http://www.smashingmagazine.com/2012/06/links-should-never-say-click-here/

i also think the experience would be greatly improved if the page was blank and the form was built and submitted in javascript (to avoid the flash of the intermediate page).  you can use <noscript> to include the existing html page and form as a fallback (including the header and footer).

however my largest concern with this intermediate page is it breaks clicking on 'back' after viewing an attachment to return to the bug.  instead you're returned to the redirect page, and then back at the attachment.  i suspect this can be fixed by including a hidden field which is set when we perform the redirect.  if the page is subsequently loaded with that hidden field populated, then we redirect to the bug instead.

Comment 32

[Gervase Markham [:gerv]]

Glob does raise some reasonable issues. I know some of them can be worked around, but it does make me wonder whether tying the token to the IP address would be a less intrusive solution. Do we think that provide sufficient security?

Gerv

Comment 33

[Frédéric Buclin]

I agree that glob raised good points. I'm personally not a fan of the intermediate page, but that's the only way I found to do a POST redirect. Binding a token to the user IP address would be less invasive. This wouldn't 100% fix the issue in case you share the same IP address as the attacker, but the probability is much lower.

Mario says this issue affects Firefox only, so a better way to fix it would be to fix bug 1189271 instead. Unfortunately, I cannot access that bug, so no idea if it got any traction. If it does, I would prefer to close this bug as WFM once a new Firefox version is available.

Comment 34

[Frédéric Buclin]

If we bind the token to the IP address, wouldn't it still be possible to the evil script executed by the victim to re-run a request with the new URL (the one with the token) to get the attachment content itself and then pass this content to the attacker? This means that the attacker would still be unable to access the attachment itself, but the evil script would have done it on behalf of the victim. Mario?

Comment 35

[Gervase Markham [:gerv]]

LpSolit: the same-origin policy would stop that, if every bug's attachment is at a different hostname (as in the BMO deployment, and as is best practice). If you are using one domain name for all attachments, I agree that this would be a problem.

Gerv

Comment 36

[Gervase Markham [:gerv]]

Bug 1189271 was only filed 24 hours ago, and as yet has no comments. I've CCed you. I'll mention the bug on the Mozilla security-group mailing list.

Gerv

Comment 37

[Frédéric Buclin]

(In reply to Gervase Markham [:gerv] from comment #35)
> LpSolit: the same-origin policy would stop that, if every bug's attachment
> is at a different hostname (as in the BMO deployment, and as is best
> practice). If you are using one domain name for all attachments, I agree
> that this would be a problem.

Using the same domain name for all attachments is pretty common.

Comment 38

[Gervase Markham [:gerv]]

But if you use the same domain name for all attachments, then any attachment can look at any other attachment just using XMLHttpRequest - there's no security barrier at all. So attachments can't steal your Bugzilla login cookies, but they can steal the contents of any other attachment.

If you are a malicious attachment, executing code in the context of http://mybzattachments.com?attachment.cgi?id=456, then you can just do:

<iframe src="http://mybugzilla.com/attachment.cgi?id=123">
which correctly authorizes itself using the user's cookies and then redirects to:
<iframe src="http://mybzattachments.com/attachment.cgi?id=123&token=sometoken">
which is then same-origin with you, and you can just read the data. So no need for an exploit at all.

AFAICS, anyway.

Gerv

Comment 39

[Mario Gomes]

(In reply to Gervase Markham [:gerv] from comment #36)
> Bug 1189271 was only filed 24 hours ago, and as yet has no comments. I've
> CCed you. I'll mention the bug on the Mozilla security-group mailing list.

All the browsers are vulnerable to cookie bombing, the difference is that Firefox is the only one where I can get the final URL through convincing the victim to drag the image :)

Comment 40

[Frédéric Buclin]

(In reply to Mario Gomes from comment #39)
> All the browsers are vulnerable to cookie bombing, the difference is that
> Firefox is the only one where I can get the final URL through convincing the
> victim to drag the image :)

What's different compared to other browers? Firefox is too verbose in its error message?

Comment 41

[Mario Gomes]

(In reply to Frédéric Buclin from comment #40)
> What's different compared to other browers? Firefox is too verbose in its
> error message?

They all have the same behave save some many cookies and send to the server causing the error messages.

In other browsers, when a image is pointing to a page that contains a redirect it won't be followed, which means: 
open <img src=https://bugzilla.mozilla.org/attachment.cgi?id=8641350></img> on chrome, then drag it to the address bar you'll get the url "https://bugzilla.mozilla.org/attachment.cgi?id=8641350" which means the browser won't provide the final URL after some many redirects(browser followed the redirects, but the only url you'll get by drag it is the one you first set).

Differently, Firefox gives us the last URL when you drag, that's why i'm only able to reproduce on firefox, it is the only browser where I can get the final URL.

Comment 42

[Frédéric Buclin]

(In reply to Mario Gomes from comment #41)
> Differently, Firefox gives us the last URL when you drag, that's why i'm
> only able to reproduce on firefox, it is the only browser where I can get
> the final URL.

So one could say that it's a bug in Firefox. :)

Comment 43

[Frédéric Buclin]

Comment on attachment 8641350 [details] [diff] [review]
patch for trunk, v1

I think the IP address binding is much less invasive than a POST redirect. The problem is that it would imply a DB schema change, so not nice for stable branches (but it's a new column, not altering an existing column, so this shouldn't break customizations).

Comment 44

[Gervase Markham [:gerv]]

Why would it be a DB schema change? We already have issue_hash_token which issues tokens which can be bound to IP addresses... What have I missed?

Gerv

Comment 45

[Frédéric Buclin]

(In reply to Gervase Markham [:gerv] from comment #44)
> Why would it be a DB schema change? We already have issue_hash_token which
> issues tokens which can be bound to IP addresses... What have I missed?

Hash tokens are not stored in the DB. Session tokens are.

Comment 46

[Gervase Markham [:gerv]]

I've filed bug 1189814 on the behaviour of drag-drop of images in Firefox exposing the final rather than the initial URL. Anne VK agrees that this is a bug and needs to be fixed.

Gerv

Comment 47

[Mario Gomes]

(In reply to Gervase Markham [:gerv] from comment #46)
> I've filed bug 1189814 on the behaviour of drag-drop of images in Firefox
> exposing the final rather than the initial URL. Anne VK agrees that this is
> a bug and needs to be fixed. 

Would you CC me in that bug ?

Comment 48

[Mario Gomes]

Is this bug been treated as a Firefox bug? Or there'll actually be a patch for this in Bugzilla ?

Comment 49

[Gervase Markham [:gerv]]

Mario: I think one aspect is (probably) a Firefox bug, but we will probably also make a fix to Bugzilla to IP-lock these tokens for extra safety. (Also, someone testing this also turned up a problem with the BMO load balancer configuration... bugs are shaking out all over the place :-)

Gerv

Comment 50

[Mario Gomes]

It's been over a month since this bug was reported and no final fix was applied so far. Is this been treated as severe as it deserves? Waiting for a patch from Firefox  side isn't a wise decision since it seems like it'll take a while until they actually have a look at it and finally fix it. 

Meanwhile, Bugzilla might still being harmed. Just like bug 953110 reported 6 months ago which is still waiting for a Adobe patch, leaving Bugzilla vulnerable. 

I suggest implement your on patches(even though this isn't exactly a Bugzilla vulnerability) than waiting for vendors to fix it, they usually take a long to do it and leave your user's in risk.

Comment 51

[Gervase Markham [:gerv]]

(In reply to Frédéric Buclin from comment #45)
> (In reply to Gervase Markham [:gerv] from comment #44)
> > Why would it be a DB schema change? We already have issue_hash_token which
> > issues tokens which can be bound to IP addresses... What have I missed?
> 
> Hash tokens are not stored in the DB. Session tokens are.

Can you clear the fog of my ignorance a bit more? Are you saying that we can't use the standard implementation of hash tokens, because they aren't stored in the DB, and they would need to be for this use? So we have to change the column where we store the token, as it's not big enough for a hash token? Or something else?

Gerv

Comment 52

[Frédéric Buclin]

(In reply to Gervase Markham [:gerv] from comment #51)
> Can you clear the fog of my ignorance a bit more? Are you saying that we
> can't use the standard implementation of hash tokens, because they aren't
> stored in the DB, and they would need to be for this use? So we have to
> change the column where we store the token, as it's not big enough for a
> hash token? Or something else?

We don't want to store hash tokens in the DB. If you need such tokens, you have sessions tokens. But in our case, we want none of them. We just want an additional column in the tokens table to store the IP address.

Comment 53

[Mario Gomes]

I'd like suggest another patch, once this vulnerability is only exploitable due to a Firefox(which seems like will take a while to be fixed) and the fact that the final URL is the only one that I get through this attack. Why don't you change the final URL? 

Do something like: 
https://bugzilla.mozilla.org/attachment.cgi?id=[attachment-ID] -> redirects to 
https://[BUG-ID].bmoattachments.org/attachment.cgi?id=[attachment-ID]&t=[TOKEN] -> now save the token as a cookie and redirect to  
https://[BUG-ID].bmoattachments.org/attachmtent.cgi?id=[attachment-ID] --> now you erase the cookie that is the token and display the attachment.

By this way there won't be anyway for getting the cookie and the token won't be displayed in the URL.

Comment 54

[Frédéric Buclin]

Extra redirects should be avoided as much as possible. They take ressources and slow down the display of the page.

Comment 55

[Mario Gomes]

Couple more redirects wont make much difference. That seems to be the easier patch. I tried it on a burq suit script and it didnt make it that much slower.

Comment 56

[Mario Gomes]

Couple more redirects wont make much difference. That seems to be the easier patch. I tried it on a burq suit script and it didnt make it that much slower.

Comment 57

[Gervase Markham [:gerv]]

(In reply to Frédéric Buclin from comment #54)
> They take resources and slow down the display of the page.

If redirects such as the ones Mario proposes take significant resources, or slow down page display beyond 2xRTT + a small epsilon, then that's a performance bug in Bugzilla.

We might decide 2xRTT is too big a price to pay (although it would only be on secure attachments, of course) but it shouldn't be more than that.

BTW, it seems from the docs:
https://developer.mozilla.org/en-US/docs/Web/HTTP/X-Frame-Options
that X-Frame-Options: DENY only applies to <frame>, <iframe> or <object>. So it wouldn't prevent the current attack, where the attachment is loaded in <img src="">. So we can't use that to do option 4 in comment 3.

The other thing we could do is look at the referrer when displaying HTML or other active content attachments, and see if it's using correct_urlbase(). If it is, we know this is a valid embedding on attachment.cgi. If it's not (e.g. it's using attachmentdomain), we know it isn't, and can refuse the load or send the data as text/plain. This requires the browser to be sending referrers for cross-domain HTTPS-to-HTTPS navigations. It also means it might break some multi-file testcases that people have assembled by uploading the resources and then uploading an HTML page which references them.

Any thoughts about that?

Gerv

Comment 58

[Frédéric Buclin]

We don't want to use X-Frame-Options: DENY. We use SAMEORIGIN on purpose. Also, I don't like playing with the referrer header. It's known to be untrustable. You can delete it, you can spoof it. We shouldn't make our code more complex than necessary. The content type sniffing of IE 6 and 7 and Safari on Windows are already responsible for some complex code in Attachment.pm to deliver attachments in a secure way. We should limit these complexities as much as possible.

Coming back to Mario's comment 53. I think the extra redirection could be avoided using the same trick as when updating a bug. process_bug.cgi is replaced by show_bug.cgi in the URL thanks to history.replaceState. I will investigate if we can do something similar here.

Comment 59

[Frédéric Buclin]

(In reply to Frédéric Buclin from comment #58)
> Coming back to Mario's comment 53. I think the extra redirection could be
> avoided using the same trick as when updating a bug. process_bug.cgi is
> replaced by show_bug.cgi in the URL thanks to history.replaceState. I will
> investigate if we can do something similar here.

Ah no, of course not we cannot. history.replaceState requires JS, but the plan here is to display raw data.

Comment 60

[Mario Gomes]

(In reply to Frédéric Buclin from comment #59)
> Ah no, of course not we cannot. history.replaceState requires JS, but the
> plan here is to display raw data.

Why don't you instead of using a 302 redirect use a "refresh" redirect?
Just like:
============
<?php
header('refresh: 0;URL=https://[BUG-ID].bmoattachments.org/attachment.cgi?id=[ATTACHMENT-ID]&t=[SEC-TOKEN]');
?>
============

<IMG> tag won't follow "refresh" redirects and they are as fast as 302 redirect on Firefox(on chrome it has some delay, you might wanna use this redirect only Firefox).

I think this is the easier and safer patch for this vulnerability.

Comment 61

[Frédéric Buclin]

Now that Firefox fixed the problem itself, there is no need for Bugzilla to do another redirect or to record the IP address. There is no need to add more complexity if this is no longer exploitable. The security flag can be removed and this bug closed once Firefox 41 (and SeaMonkey 2.38?) is released.

Comment 62

[Gervase Markham [:gerv]]

Well, I think restricting these tokens to the same IP address is still a wise "belt and braces" move. The other options all have potential compatibility issues but I don't think that one does. Does it?

Gerv

Comment 63

[Frédéric Buclin]

(In reply to Gervase Markham [:gerv] from comment #62)
> Well, I think restricting these tokens to the same IP address is still a
> wise "belt and braces" move.

This bug is already full of comments and is about a different issue. I much prefer to file a separate enhancement bug than to keep working on this one.

Comment 64

[Gervase Markham [:gerv]]

Bug 1198028 filed to consider that change.

Gerv

Comment 65

[Frédéric Buclin]

(In reply to Frédéric Buclin from comment #61)
> The security flag can be removed and this bug closed once Firefox 41 is released.