WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
113387
X-Frame-Options: Multiple headers are ignored completely.
https://bugs.webkit.org/show_bug.cgi?id=113387
Summary
X-Frame-Options: Multiple headers are ignored completely.
Mike West
Reported
2013-03-27 04:02:11 PDT
http://www.veracode.com/blog/2013/03/security-headers-on-the-top-1000000-websites-march-2013-report/
notes that a small number of sites send multiple 'X-Frame-Options' headers. We're currently ignoring those completely, which probably isn't the safest behavior. Gecko has settled on parsing each header, using the value if they're all the same, and defaulting to 'DENY' if they conflict, which seems like a better solution[1]. I'd suggest that WebKit do the same. [1]:
https://bugzilla.mozilla.org/show_bug.cgi?id=761655
Attachments
Patch
(23.26 KB, patch)
2013-03-27 06:24 PDT
,
Mike West
no flags
Details
Formatted Diff
Diff
Patch for landing
(23.27 KB, patch)
2013-03-28 01:32 PDT
,
Mike West
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Mike West
Comment 1
2013-03-27 06:15:57 PDT
Also filed downstream as
https://code.google.com/p/chromium/issues/detail?id=145659
.
Mike West
Comment 2
2013-03-27 06:24:52 PDT
Created
attachment 195300
[details]
Patch
Mike West
Comment 3
2013-03-27 10:29:35 PDT
Bots seem happy. Nate, would you mind taking a look at this?
Nate Chapin
Comment 4
2013-03-27 10:39:01 PDT
Comment on
attachment 195300
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=195300&action=review
One style nit...
> Source/WebCore/loader/FrameLoader.cpp:2984 > + default: > + m_frame->document()->addConsoleMessage(JSMessageSource, ErrorMessageLevel, "Invalid 'X-Frame-Options' header encountered when loading '" + url.elidedString() + "': '" + content + "' is not a recognized directive. The header will be ignored.", requestIdentifier); > + return false;
I think it's more common in WebKit (or at least the parts I frequent) to explicitly state all cases and have the default be ASSERT_NOT_REACHED().
Mike West
Comment 5
2013-03-28 01:32:45 PDT
Created
attachment 195500
[details]
Patch for landing
Mike West
Comment 6
2013-03-28 01:33:32 PDT
(In reply to
comment #4
)
> (From update of
attachment 195300
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=195300&action=review
> > One style nit... > > > Source/WebCore/loader/FrameLoader.cpp:2984 > > + default: > > + m_frame->document()->addConsoleMessage(JSMessageSource, ErrorMessageLevel, "Invalid 'X-Frame-Options' header encountered when loading '" + url.elidedString() + "': '" + content + "' is not a recognized directive. The header will be ignored.", requestIdentifier); > > + return false; > > I think it's more common in WebKit (or at least the parts I frequent) to explicitly state all cases and have the default be ASSERT_NOT_REACHED().
Thanks Nate. I've taken care of that in the patch up for the CQ.
WebKit Review Bot
Comment 7
2013-03-28 01:57:16 PDT
Comment on
attachment 195500
[details]
Patch for landing Clearing flags on attachment: 195500 Committed
r147086
: <
http://trac.webkit.org/changeset/147086
>
WebKit Review Bot
Comment 8
2013-03-28 01:57:20 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug