Commit 6d6d84b2 authored by Takashi Toyoshima's avatar Takashi Toyoshima Committed by Commit Bot

Network Service: Add method validation as the secondary check

Today, the method valid of the HTTP request is validated in
Blink for user exposed APIs such as Fetch and XHR to conform
the RFC 7230. But it's still possible that compromised
renderers insert arbitrary ASCII strings to the method value.

This patch adds the same RFC 7230 token check in the network
service as the secondary check.

Bug: 1110195
Change-Id: Ia99a986f82034875f7e8c0b2224f2260a99eeffa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2355534Reviewed-by: default avatarMatt Menke <mmenke@chromium.org>
Commit-Queue: Takashi Toyoshima <toyoshim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#798557}
parent 5881529d
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
#include "mojo/public/cpp/bindings/message.h" #include "mojo/public/cpp/bindings/message.h"
#include "mojo/public/cpp/bindings/remote.h" #include "mojo/public/cpp/bindings/remote.h"
#include "net/base/load_flags.h" #include "net/base/load_flags.h"
#include "net/http/http_util.h"
#include "services/network/cors/cors_url_loader.h" #include "services/network/cors/cors_url_loader.h"
#include "services/network/cors/preflight_controller.h" #include "services/network/cors/preflight_controller.h"
#include "services/network/crash_keys.h" #include "services/network/crash_keys.h"
...@@ -461,6 +462,13 @@ bool CorsURLLoaderFactory::IsValidRequest(const ResourceRequest& request, ...@@ -461,6 +462,13 @@ bool CorsURLLoaderFactory::IsValidRequest(const ResourceRequest& request,
return false; return false;
} }
if (!net::HttpUtil::IsToken(request.method)) {
// Callers are expected to ensure that |method| follows RFC 7230.
mojo::ReportBadMessage(
"CorsURLLoaderFactory: invalid characters in method");
return false;
}
// TODO(yhirano): If the request mode is "no-cors", the redirect mode should // TODO(yhirano): If the request mode is "no-cors", the redirect mode should
// be "follow". // be "follow".
return true; return true;
......
...@@ -466,6 +466,28 @@ class BadMessageTestHelper { ...@@ -466,6 +466,28 @@ class BadMessageTestHelper {
DISALLOW_COPY_AND_ASSIGN(BadMessageTestHelper); DISALLOW_COPY_AND_ASSIGN(BadMessageTestHelper);
}; };
TEST_F(CorsURLLoaderTest, NoCorsWithInvalidMethod) {
ResourceRequest request;
request.mode = mojom::RequestMode::kNoCors;
request.credentials_mode = mojom::CredentialsMode::kInclude;
request.url = GURL("http://example.com/");
request.request_initiator = base::nullopt;
request.method = "GET\r\nHost: other.example.com";
BadMessageTestHelper bad_message_helper;
CreateLoaderAndStart(request);
RunUntilComplete();
EXPECT_FALSE(IsNetworkLoaderStarted());
EXPECT_FALSE(client().has_received_redirect());
EXPECT_FALSE(client().has_received_response());
EXPECT_TRUE(client().has_received_completion());
EXPECT_EQ(net::ERR_INVALID_ARGUMENT, client().completion_status().error_code);
EXPECT_THAT(bad_message_helper.bad_message_reports(),
::testing::ElementsAre(
"CorsURLLoaderFactory: invalid characters in method"));
}
TEST_F(CorsURLLoaderTest, SameOriginWithoutInitiator) { TEST_F(CorsURLLoaderTest, SameOriginWithoutInitiator) {
ResourceRequest request; ResourceRequest request;
request.mode = mojom::RequestMode::kSameOrigin; request.mode = mojom::RequestMode::kSameOrigin;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment