Respect the "allowall" value for the X-Frame-Options header, as some

front-ends send this rather than simply omitting the X-Frame-Options
header altogether.

BUG=none
TEST=chrome_frame_unittests.exe

Review URL: http://codereview.chromium.org/404003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@32473 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 106 insertions, 15 deletions

diff --git a/chrome_frame/chrome_frame.gyp b/chrome_frame/chrome_frame.gyp
index b6a144f..870f9da 100644
--- a/chrome_frame/chrome_frame.gyp
+++ b/chrome_frame/chrome_frame.gyp
@@ -233,6 +233,7 @@
         'chrome_frame_npapi_unittest.cc',
         'chrome_frame_unittest_main.cc',
         'chrome_launcher_unittest.cc',
+        'html_utils_unittest.cc',
         'unittest_precompile.h',
         'unittest_precompile.cc',
         'urlmon_upload_data_stream.cc',
diff --git a/chrome_frame/html_utils.cc b/chrome_frame/html_utils.cc
index 7ab1fd1..e964ab9 100644
--- a/chrome_frame/html_utils.cc
+++ b/chrome_frame/html_utils.cc
@@ -12,6 +12,8 @@
 #include "chrome_frame/utils.h"
 
 const wchar_t kQuotes[] = L"\"'";
+const char kXFrameOptionsHeader[] = "X-Frame-Options";
+const char kXFrameOptionsValueAllowAll[] = "allowall";
 
 HTMLScanner::StringRange::StringRange() {
 }
@@ -352,4 +354,22 @@ std::string GetDefaultUserAgent() {
   return ret;
 }
 
+bool HasFrameBustingHeader(const std::string& http_headers) {
+  net::HttpUtil::HeadersIterator it(
+      http_headers.begin(), http_headers.end(), "\r\n");
+  while (it.GetNext()) {
+    if (it.name() == kXFrameOptionsHeader) {
+      std::string allow_all(kXFrameOptionsValueAllowAll);
+      if (it.values_end() - it.values_begin() != allow_all.length() ||
+          !std::equal(it.values_begin(), it.values_end(),
+              allow_all.begin(),
+              CaseInsensitiveCompareASCII<const char>())) {
+        return true;
+      }
+    }
+  }
+
+  return false;
+}
+
 }  // namespace http_utils
diff --git a/chrome_frame/html_utils.h b/chrome_frame/html_utils.h
index 369950c..daface10 100644
--- a/chrome_frame/html_utils.h
+++ b/chrome_frame/html_utils.h
@@ -139,6 +139,10 @@ std::string GetDefaultUserAgent();
 // table not being present in the unit test executable.
 const char* GetChromeFrameUserAgent();
 
+// Returns true if there is a frame busting header (other than the do-nothing
+// "X-Frame-Options: ALLOWALL") in the provided header block.
+bool HasFrameBustingHeader(const std::string& http_headers);
+
 }  // namespace http_utils
 
 #endif  // CHROME_FRAME_HTML_UTILS_H_
diff --git a/chrome_frame/html_utils_unittest.cc b/chrome_frame/html_utils_unittest.cc
new file mode 100644
index 0000000..73b7a4a
--- /dev/null
+++ b/chrome_frame/html_utils_unittest.cc
@@ -0,0 +1,61 @@
+// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/logging.h"
+#include "chrome_frame/html_utils.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+TEST(HttpUtils, HasFrameBustingHeader) {
+  // Simple negative cases.
+  ASSERT_FALSE(http_utils::HasFrameBustingHeader(""));
+  ASSERT_FALSE(http_utils::HasFrameBustingHeader("Content-Type: text/plain"));
+  // Explicit negative cases, test that we ignore case.
+  ASSERT_FALSE(http_utils::HasFrameBustingHeader("X-Frame-Options: ALLOWALL"));
+  ASSERT_FALSE(http_utils::HasFrameBustingHeader("X-Frame-Options: allowall"));
+  ASSERT_FALSE(http_utils::HasFrameBustingHeader("X-Frame-Options: ALLowalL"));
+  // Added space, ensure stripped out
+  ASSERT_FALSE(http_utils::HasFrameBustingHeader(
+      "X-Frame-Options: ALLOWALL "));
+  // Added space with linefeed, ensure still stripped out
+  ASSERT_FALSE(http_utils::HasFrameBustingHeader(
+      "X-Frame-Options: ALLOWALL \r\n"));
+  // Multiple identical headers, all of them allowing framing.
+  ASSERT_FALSE(http_utils::HasFrameBustingHeader(
+      "X-Frame-Options: ALLOWALL\r\n"
+      "X-Frame-Options: ALLOWALL\r\n"
+      "X-Frame-Options: ALLOWALL"));
+  // Interleave with other headers.
+  ASSERT_FALSE(http_utils::HasFrameBustingHeader(
+      "Content-Type: text/plain\r\n"
+      "X-Frame-Options: ALLOWALL\r\n"
+      "Content-Length: 42"));
+
+  // Simple positive cases.
+  ASSERT_TRUE(http_utils::HasFrameBustingHeader("X-Frame-Options: deny"));
+  ASSERT_TRUE(http_utils::HasFrameBustingHeader(
+      "X-Frame-Options: SAMEorigin"));
+
+  // Allowall entries do not override the denying entries, are
+  // order-independent, and the deny entries can interleave with
+  // other headers.
+  ASSERT_TRUE(http_utils::HasFrameBustingHeader(
+      "Content-Length: 42\r\n"
+      "X-Frame-Options: ALLOWall\r\n"
+      "X-Frame-Options: deny\r\n"));
+  ASSERT_TRUE(http_utils::HasFrameBustingHeader(
+      "X-Frame-Options: ALLOWall\r\n"
+      "Content-Length: 42\r\n"
+      "X-Frame-Options: SAMEORIGIN\r\n"));
+  ASSERT_TRUE(http_utils::HasFrameBustingHeader(
+      "X-Frame-Options: deny\r\n"
+      "X-Frame-Options: ALLOWall\r\n"
+      "Content-Length: 42\r\n"));
+  ASSERT_TRUE(http_utils::HasFrameBustingHeader(
+      "X-Frame-Options: SAMEORIGIN\r\n"
+      "X-Frame-Options: ALLOWall\r\n"));
+}
+
+}  // namespace
diff --git a/chrome_frame/urlmon_url_request.cc b/chrome_frame/urlmon_url_request.cc
index 6ca2401..71aee9b 100644
--- a/chrome_frame/urlmon_url_request.cc
+++ b/chrome_frame/urlmon_url_request.cc
@@ -11,6 +11,7 @@
 #include "base/logging.h"
 #include "base/message_loop.h"
 #include "chrome_frame/chrome_frame_activex_base.h"
+#include "chrome_frame/html_utils.h"
 #include "chrome_frame/urlmon_upload_data_stream.h"
 #include "chrome_frame/utils.h"
 #include "net/http/http_util.h"
@@ -19,7 +20,6 @@
 static const LARGE_INTEGER kZero = {0};
 static const ULARGE_INTEGER kUnsignedZero = {0};
 int UrlmonUrlRequest::instance_count_ = 0;
-const char kXFrameOptionsHeader[] = "X-Frame-Options";
 
 UrlmonUrlRequest::UrlmonUrlRequest()
     : pending_read_size_(0),
@@ -400,23 +400,28 @@ STDMETHODIMP UrlmonUrlRequest::OnResponse(DWORD dwResponseCode,
 
   // Security check for frame busting headers. We don't honor the headers
   // as-such, but instead simply kill requests which we've been asked to
-  // look for. This puts the onus on the user of the UrlRequest to specify
-  // whether or not requests should be inspected. For ActiveDocuments, the
-  // answer is "no", since WebKit's detection/handling is sufficient and since
-  // ActiveDocuments cannot be hosted as iframes. For NPAPI and ActiveX
-  // documents, the Initialize() function of the PluginUrlRequest object
-  // allows them to specify how they'd like requests handled. Both should
-  // set enable_frame_busting_ to true to avoid CSRF attacks.
-  // Should WebKit's handling of this ever change, we will need to re-visit
-  // how and when frames are killed to better mirror a policy which may
-  // do something other than kill the sub-document outright.
+  // look for if they specify a value for "X-Frame-Options" other than
+  // "ALLOWALL" (the others are "deny" and "sameorigin"). This puts the onus
+  // on the user of the UrlRequest to specify whether or not requests should
+  // be inspected. For ActiveDocuments, the answer is "no", since WebKit's
+  // detection/handling is sufficient and since ActiveDocuments cannot be
+  // hosted as iframes. For NPAPI and ActiveX documents, the Initialize()
+  // function of the PluginUrlRequest object allows them to specify how they'd
+  // like requests handled. Both should set enable_frame_busting_ to true to
+  // avoid CSRF attacks. Should WebKit's handling of this ever change, we will
+  // need to re-visit how and when frames are killed to better mirror a policy
+  // which may do something other than kill the sub-document outright.
 
   // NOTE(slightlyoff): We don't use net::HttpResponseHeaders here because
   //    of lingering ICU/base_noicu issues.
-  if (frame_busting_enabled_ &&
-      net::HttpUtil::HasHeader(raw_headers, kXFrameOptionsHeader)) {
-    DLOG(ERROR) << "X-Frame-Options header detected, navigation canceled";
-    return E_FAIL;
+  if (frame_busting_enabled_) {
+    std::string http_headers = net::HttpUtil::AssembleRawHeaders(
+        raw_headers.c_str(), raw_headers.length());
+    if (http_utils::HasFrameBustingHeader(http_headers)) {
+      DLOG(ERROR) << "X-Frame-Options header other than ALLOWALL " <<
+          "detected, navigation canceled";
+      return E_FAIL;
+    }
   }
 
   std::wstring url_for_persistent_cookies =