Closed
Bug 475894
Opened 16 years ago
Closed 14 years ago
Consider sending X-FRAME-OPTIONS: SAMEORIGIN for ClickJacking prevention
Categories
(Bugzilla :: Bugzilla-General, enhancement)
Tracking
()
RESOLVED
FIXED
Bugzilla 4.0
People
(Reporter: mkanat, Assigned: reed)
References
()
Details
(Whiteboard: [wanted-bmo][infrasec:crossdomain][ws:moderate])
Attachments
(1 file, 3 obsolete files)
(deleted),
patch
|
mkanat
:
review+
|
Details | Diff | Splinter Review |
IE 8 is going to have a new feature that helps prevent clickjacking, which prevents a site from being rendered in a frame with various restrictions. It's an HTTP header, X-Frame-Options. We could set it to "SAMEORIGIN" and then only Bugzilla could frame itself.
I'm not sure if this is a good idea or if it will become any sort of web standard, but I figured I could file this bug and we could discuss it.
Details at http://blogs.msdn.com/ie/archive/2009/01/27/ie8-security-part-vii-clickjacking-defenses.aspx
Assignee | ||
Comment 1•16 years ago
|
||
See bug 475530 for discussion on implementation (or not) of the header into Gecko.
Updated•15 years ago
|
Blocks: q2-review-bmo
Comment 2•15 years ago
|
||
I'd like to discuss this issue a bit more. FF is going to support x-frame-options and IE8 already does. It would be a security win for bugzilla to prevent click-jacking attacks via x-frame-options. In fact, a clickjacking attack was demo'ed against bugzilla in BlackHat europe to bypass CSRF controls.
Thoughts?
Reporter | ||
Comment 3•15 years ago
|
||
Well, I'm for it. If somebody wants to provide a patch, I'll be happy to review it. I don't think framing Bugzilla is an important use case, and people who really have to could always edit the code.
I would like some confirmation that this is indeed a helpful protection, as some comments from dveditz in bug 475530 indicate that it might not be as helpful as it seems.
Comment 4•15 years ago
|
||
I looked through bug 475530. The main point of concern I saw was comment 13 (copied below)
<comment>
Interesting, according to Eric's comments SAMEORIGIN is compared only against
the origin of the top-level document; it ought to be checked all the way up the
frame tree. I suppose that requires more work for a rare case, but it seems
kind of half-baked if this is really a security measure and not just a
feel-good item no one is expected to actually use.
</comment>
I think this limitation is not a big issue. The most common attack scenario I see for click jacking is an unrelated site framing bugzilla. In this case the top-level origin will be the attacker's site and thus not match the framed bugzilla page. In this scenario x-frame-options would work to protect us.
The only scenario where this top-level checking would be an issue is if the attacker could inject their own frame into bugzilla and then execute the clickjacking attack within his injected frame (e.g. the top level would still be bugzilla and thus no protection from x-frame-options). However, arbitrary iframe injection into bugzilla would be a bigger deal anyway and I wouldn't expect protection from x-frame-options.
Comment 5•14 years ago
|
||
Per http://michael-coates.blogspot.com/2010/08/x-frame-option-support-in-firefox.html, IE8, Fx 3.6.9, Opera 10.50 and Safari 4 support it.
Updated•14 years ago
|
Flags: blocking4.0?
Updated•14 years ago
|
Summary: Consider sending X-FRAME-OPTIONS: SAMEORIGIN for ClickJacking prevention in IE 8 → Consider sending X-FRAME-OPTIONS: SAMEORIGIN for ClickJacking prevention
Assignee | ||
Comment 6•14 years ago
|
||
Assignee: general → reed
Status: NEW → ASSIGNED
Comment 7•14 years ago
|
||
(In reply to comment #4)
My concerns about SAMEORIGIN in bug 475530 comment 13 shouldn't be taken as discouragement from using it -- but beware its limitations and don't count on it for too much.
> I think this limitation is not a big issue. The most common attack scenario I
> see for click jacking is an unrelated site framing bugzilla. [...]
> In this scenario x-frame-options would work to protect us.
It will certainly help in this most-common case.
> The only scenario where this top-level checking would be an issue is if the
> attacker could inject their own frame into bugzilla and then execute the
> clickjacking attack within his injected frame (e.g. the top level would still
> be bugzilla and thus no protection from x-frame-options). However, arbitrary
> iframe injection into bugzilla would be a bigger deal anyway and I wouldn't
> expect protection from x-frame-options.
Generic bugzilla may not have this issue but BMO is _full_ of arbitrary frames--in testcases. In BMO the separate attachment hosts saves us, but that's a relatively recent feature. I think it's great bugzilla is planning on using this feature, my comments were simply that people need to carefully consider their entire site before relying on the SAMEORIGIN feature.
Incidentally, if the separate attachment host feature is turned on then attachments must not use the x-frame-options feature. Attachment content is displayed in a non-same-origin frame on the attachment details page.
In addition some testcases may also have one attachment frame another, possibly from another bug in rare cases. Perhaps, though, we don't care to cater to those edge cases.
Reporter | ||
Comment 8•14 years ago
|
||
I'll take this on 4.0 if we get it in before RC1, but I won't block 4.0 on it.
Flags: blocking4.0? → blocking4.0-
Target Milestone: --- → Bugzilla 4.0
Assignee | ||
Updated•14 years ago
|
Whiteboard: [wanted-bmo][sg:want]
Updated•14 years ago
|
Whiteboard: [wanted-bmo][sg:want] → [wanted-bmo][sg:want][ws:moderate]
Assignee | ||
Updated•14 years ago
|
Whiteboard: [wanted-bmo][sg:want][ws:moderate] → [wanted-bmo][infrasec:crossdomain][ws:moderate]
Assignee | ||
Comment 9•14 years ago
|
||
Ok, add the X-FRAME-OPTIONS header unless attachbase is on and the current page is attachment.cgi.
Possible issue: This does NOT send the header if attachbase is on and if you go directly to the attachbase URL. However, if attachbase is off, the header will still be sent for attachment.cgi. Should the header be sent on the attachbase itself, or do we want to permit attachments to go outside the current origin?
Attachment #472998 -
Attachment is obsolete: true
Attachment #498076 -
Flags: review?(mkanat)
Assignee | ||
Comment 10•14 years ago
|
||
I think this is right, though I would like an answer to my last comment, as it's too late for me to be thinking correctly on how this should be...
Attachment #498076 -
Attachment is obsolete: true
Attachment #498077 -
Flags: review?(mkanat)
Attachment #498076 -
Flags: review?(mkanat)
Comment 11•14 years ago
|
||
you must NOT send the header for attachbase.bugzilla.domain because then it can't be hosted in the attachment detail view page frame. without attachbase I think you can safely just send the header all the time, attachment content or not.
Assignee | ||
Comment 12•14 years ago
|
||
Send the header at all times UNLESS the current URL is part of the attachment base.
Attachment #498077 -
Attachment is obsolete: true
Attachment #498464 -
Flags: review?(mkanat)
Attachment #498077 -
Flags: review?(mkanat)
Reporter | ||
Comment 13•14 years ago
|
||
Comment on attachment 498464 [details] [diff] [review]
patch - v4
Sure, this looks great. Thanks! :-)
Attachment #498464 -
Flags: review?(mkanat) → review+
Reporter | ||
Comment 14•14 years ago
|
||
How far back does url_is_attachmentbase exist? If it exists on 3.6, we could backport to 3.6 too.
Flags: approval4.0+
Flags: approval+
Assignee | ||
Comment 15•14 years ago
|
||
Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/CGI.pm
Committed revision 7628.
Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/4.0/
modified Bugzilla/CGI.pm
Committed revision 7498.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 16•14 years ago
|
||
(In reply to comment #14)
> How far back does url_is_attachmentbase exist? If it exists on 3.6, we could
> backport to 3.6 too.
It exists on 3.6, at least. If we were to backport this, there are a few other things (such as bug 453425 and possibly bug 562475) we should backport as well.
You need to log in
before you can comment on or make changes to this bug.
Description
•