Bug 101447 - Warn when parsing an invalid X-Frame-Options header.
Summary: Warn when parsing an invalid X-Frame-Options header.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mike West
URL:
Keywords: WebExposed
Depends on:
Blocks:
 
Reported: 2012-11-07 04:45 PST by Mike West
Modified: 2012-11-08 01:56 PST (History)
3 users (show)

See Also:


Attachments
Patch (10.34 KB, patch)
2012-11-07 14:15 PST, Mike West
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mike West 2012-11-07 04:45:48 PST
The report at http://www.veracode.com/blog/2012/11/security-headers-report/ notes that ~1.7% of the ~12k sites it found to be sending X-Frame-Options headers were invalid. Currently, if we see such a header with a value that is not a case-insensitive match for SAMEORIGIN or DENY, we ignore it completely.

Interpreting an invalid X-Frame-Options header as DENY seems safer. Sites probably aren't setting an X-Frame-Options header with the intent of allowing frames (with the exception of 'Allow-From', I suppose... should we support that option?).

If that's too draconian, we should still at least throw a warning that the header is being ignored.

Adam, WDYT?
Comment 1 Adam Barth 2012-11-07 10:04:50 PST
What does the spec say to do?  http://tools.ietf.org/html/draft-ietf-websec-x-frame-options
Comment 2 Mike West 2012-11-07 12:29:13 PST
(In reply to comment #1)
> What does the spec say to do?  http://tools.ietf.org/html/draft-ietf-websec-x-frame-options

So far as I can tell, the spec is silent on the issue of invalid header content. The closest I see is "Any data beyond the domain address (i.e. any data after the "/" separator) is to be ignored." when discussing the 'Allow-From' option.

And actually, contrary to the article, it looks like Firefox exhibits the same behavior as WebKit and IE by ignoring invalid headers (assuming that I'm looking at the right code: http://mxr.mozilla.org/mozilla-aurora/source/docshell/base/nsDSURIContentListener.cpp#262).

So, why don't we just send a warning, but keep the behavior the same as Firefox and IE?

Implementing 'Allow-From' is a separate question. I'd lean towards yes. The functionality seems useful (and conceptually similar to CORS), it's specified, IE supports it, and it looks like Mozilla is playing with it (http://mxr.mozilla.org/mozilla-central/source/docshell/base/nsDSURIContentListener.cpp#375 (Is "Mozilla Central" older or newer than "Mozilla Aurora"?)). If you think it's reasonable to implement in WebKit, I'll put a patch together in a separate bug.
Comment 3 Adam Barth 2012-11-07 13:28:47 PST
I don't think we should implement allow-from.  I should email the working group and complain about its inclusion in the spec.
Comment 4 Mike West 2012-11-07 13:31:17 PST
(In reply to comment #3)
> I don't think we should implement allow-from.  I should email the working group and complain about its inclusion in the spec.

How about the source list associated with the 'frame-options' directive in the UI Safety spec (assuming that becomes the place where frame options live)?
Comment 5 Adam Barth 2012-11-07 13:43:16 PST
Yes, the reason not to implement allow-from is because we'd like to unify the handling with CSP and not have a separate code path.

I've emailed the working group.  The spec is only informational, so it's hard to get things removed.  Instead I've asked for a note explaining that allow-from is an IE-only extension.
Comment 6 Mike West 2012-11-07 14:15:06 PST
Created attachment 172869 [details]
Patch
Comment 7 WebKit Review Bot 2012-11-08 01:43:47 PST
Comment on attachment 172869 [details]
Patch

Clearing flags on attachment: 172869

Committed r133868: <http://trac.webkit.org/changeset/133868>
Comment 8 WebKit Review Bot 2012-11-08 01:43:51 PST
All reviewed patches have been landed.  Closing bug.