Bug 101447 – Warn when parsing an invalid X-Frame-Options header.

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Mike West]

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?

[Adam Barth]

What does the spec say to do?  http://tools.ietf.org/html/draft-ietf-websec-x-frame-options

[Mike West]

(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.

[Adam Barth]

I don't think we should implement allow-from.  I should email the working group and complain about its inclusion in the spec.

[Mike West]

(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)?

[Adam Barth]

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.

[Mike West]

Created attachment 172869 [details]
Patch

[WebKit Review Bot]

Comment on attachment 172869 [details]
Patch

Clearing flags on attachment: 172869

Committed r133868: <http://trac.webkit.org/changeset/133868>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.