User Agent: Mozilla/5.0 (Windows NT 6.1) AppleWebKit/535.7 (KHTML, like Gecko) Chrome/16.0.912.75 Safari/535.7 Steps to reproduce: Hello, all hard test done in https://landfill.bugzilla.org/bugzilla-tip/show_bug.cgi?id=16947. Reproduce: 1. Open this https://landfill.bugzilla.org/bugzilla-tip/attachment.cgi?id=2346&action=edit 2. Click in "Click Me!" 3. Back to https://landfill.bugzilla.org/bugzilla-tip/show_bug.cgi?id=16947 see the changes. Tested using Chrome 16. Windows 7 SP1 Cheers, Mario.

Confirmed. Using attachment_base doesn't help.

Note that we don't exactly bypass the security token check as process_bug.cgi still asks you to confirm the changes. But the user doesn't realize that the "click me" button is to confirm the changes as the iframe is hidden. This is not a critical security bug, because a) viewing the attachment directly cannot trigger this problem, due to the implementation of X-FRAME-OPTIONS: SAMEORIGIN in bug 475894 + the alternate attachment host in bug 38862; and b) it requires a user action from the Details page, and users should understand that there is a risk to interact with an HTML document. To mitigate this last problem, we could display a warning above our iframe when the MIME type is text/html, and if the alternate attachment host is available, we could also suggest to view the attachment directly, which would annihilate this clickjacking problem. I think that in most cases, this warning should be enough. If the user ignores this warning, there is nothing we can do for him. This clickjacking problem has already been discussed in bug 26257 comment 128, bug 475894 comment 4, bug 475894 comment 7 and bug 600692 comment 9. So I'm removing the security flag as this discussion already took place there, and these bugs are public.

Hum, one easy way to fix this problem is to *always* display the source code of HTML files in the "Details" page, and ask the user to click "View" to view the HTML page itself. As I said, clickjacking is not possible from the alternate host as we use X-FRAME-OPTIONS: SAMEORIGIN. No need for the warning message I suggested in my previous comment, and no way to bypass this restriction (for users who never read warnings). I think that developers who play with HTML testcases don't do it from the Details page anyway, so they wouldn't be hurt if we display the source code instead of the HTML page itself. Marking as a blocker for 4.2 for now, because if everybody agrees with my proposal (source code), it would be very easy to do and would really worth it.

+1 from me. I think it's a good idea.

I do not agree to let this vulnerability (or bug) in public. is an easy to be exploited, and is easy to be repaired. For example, in Firefox, I can report a vulnerability, and convince someone of safety and security team of Firefox click on "Click Me!" and remove the security restrictions in a real security bug. Exemple of the report: =========================== Hello, Firefox crashes exploitable when manipulating IFRAMES. The failure appears to be exploited, but to be performed is necessary to be viewed by the "Details" in Bugzilla. Reproduce: 1. Open with firefox https://landfill.bugzilla.org/bugzilla-tip/attachment.cgi?id=2346&action=edit 2. Click in "Click Me!" 3. See the CRASH (To help convince, I can post a Stacktrace false Here.) Regards, Mario ======================== Using this, I have great chances of the exploit work and changes are made in a security bug. (In reply to Frédéric Buclin from comment #3) > Note that we don't exactly bypass the security token check as > process_bug.cgi still asks you to confirm the changes. But the user doesn't > realize that the "click me" button is to confirm the changes as the iframe > is hidden. > > This is not a critical security bug, because a) viewing the attachment > directly cannot trigger this problem, due to the implementation of > X-FRAME-OPTIONS: SAMEORIGIN in bug 475894 + the alternate attachment host in > bug 38862; and b) it requires a user action from the Details page, and users > should understand that there is a risk to interact with an HTML document. To > mitigate this last problem, we could display a warning above our iframe when > the MIME type is text/html, and if the alternate attachment host is > available, we could also suggest to view the attachment directly, which > would annihilate this clickjacking problem. I think that in most cases, this > warning should be enough. If the user ignores this warning, there is nothing > we can do for him. > > This clickjacking problem has already been discussed in bug 26257 comment > 128, bug 475894 comment 4, bug 475894 comment 7 and bug 600692 comment 9. So > I'm removing the security flag as this discussion already took place there, > and these bugs are public.

(In reply to Mario Gomes from comment #6) > I do not agree to let this vulnerability (or bug) in public. This discussion already took place for a long time. There is no reason to keep this bug private. > report a vulnerability, and convince someone of safety and security team of > Firefox click on "Click Me!" and remove the security restrictions in a real > security bug. I doubt members of the security team are so easily abused by such statements. They are the ones who mentioned clickjacking problems in bugs I mentioned in comment 3.

So I think you can close this as "duplicate". Doubt, why not do a test?

(In reply to Frédéric Buclin from comment #4) > Hum, one easy way to fix this problem is to *always* display the source code > of HTML files in the "Details" page, and ask the user to click "View" to > view the HTML page itself. that sounds sane.

+1 on showing the source on the details page. I've been pretty annoyed in the past at having to load HTML that did weird things when just trying to edit details about the attachment.

Do we want it for 3.6.8 too? Not sure the 8th minor release is the best moment to implement it. It may be confusing to some users. But if we decide to take it for 3.6.8, we should also take the X-FRAME-OPTIONS patch from bug 475894 for a complete fix.

This solution works for as well. dkl

Comment on attachment 587154 [details] [diff] [review] patch, v1 Review of attachment 587154 [details] [diff] [review]: ----------------------------------------------------------------- Looks fine and works as expected. r=dkl

We should relnote it for 4.2 too, when releasing 4.2 final (or 4.2rc2, if there is one).

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/ modified template/en/default/attachment/edit.html.tmpl modified template/en/default/global/textarea.html.tmpl Committed revision 8067. Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.2/ modified template/en/default/attachment/edit.html.tmpl modified template/en/default/global/textarea.html.tmpl Committed revision 8001. Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.0/ modified template/en/default/attachment/edit.html.tmpl modified template/en/default/global/textarea.html.tmpl Committed revision 7680.

For the record, that's the patch for 4.0.4 with bitrot fixed. This patch doesn't apply to 3.6.7, so do not try to use it there.

Added to relnotes for 4.2rc2 and 4.0.4.

Remains vulnerable when you open SVG and XHTML.

(In reply to Mario Gomes from comment #19) > Remains vulnerable when you open SVG and XHTML. Do you have a PoC for SVG, please? :)

Yep, the PoC can be seen here https://landfill.bugzilla.org/bugzilla-tip/attachment.cgi?id=2466&action=edit.

Instead of a blacklist, I think it makes sense to go with a whitelist (text/plain, image/*, application/pdf [ugh]).

(In reply to Reed Loden [:reed] (very busy) from comment #22) > Instead of a blacklist, I think it makes sense to go with a whitelist > (text/plain, image/*, application/pdf [ugh]). But not image/svg+xml? ;)

(In reply to Jesse Ruderman from comment #23) > But not image/svg+xml? ;) Heh, good point! :) Please file a separate bug about the possible use of a whitelist, with Mario's URL from comment 21. But that's not something we will backport to branches. Clickjacking is not as bad as CSRF.