Commit 7d87494c authored by Julia Tuttle's avatar Julia Tuttle Committed by Commit Bot

Reporting: Limit JSON size and depth.

is makes it less likely that origins will be able to exploit
base::JSONReader using Report-To headers.

Bug: 810142
Change-Id: Ie27d98efe2afbfbeec2e4767e2a64b546e4483d9
Reviewed-on: https://chromium-review.googlesource.com/942231Reviewed-by: default avatarJulia Tuttle <juliatuttle@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540395}
parent cbd2ab58
...@@ -12,6 +12,9 @@ namespace net { ...@@ -12,6 +12,9 @@ namespace net {
namespace { namespace {
const int kMaxJsonSize = 16 * 1024;
const int kMaxJsonDepth = 5;
class ReportingDelegateImpl : public ReportingDelegate { class ReportingDelegateImpl : public ReportingDelegate {
public: public:
ReportingDelegateImpl(URLRequestContext* request_context) ReportingDelegateImpl(URLRequestContext* request_context)
...@@ -53,7 +56,13 @@ class ReportingDelegateImpl : public ReportingDelegate { ...@@ -53,7 +56,13 @@ class ReportingDelegateImpl : public ReportingDelegate {
void ParseJson(const std::string& unsafe_json, void ParseJson(const std::string& unsafe_json,
const JsonSuccessCallback& success_callback, const JsonSuccessCallback& success_callback,
const JsonFailureCallback& failure_callback) const override { const JsonFailureCallback& failure_callback) const override {
std::unique_ptr<base::Value> value = base::JSONReader::Read(unsafe_json); if (unsafe_json.size() > kMaxJsonSize) {
failure_callback.Run();
return;
}
std::unique_ptr<base::Value> value = base::JSONReader::Read(
unsafe_json, base::JSON_PARSE_RFC, kMaxJsonDepth);
if (value) if (value)
success_callback.Run(std::move(value)); success_callback.Run(std::move(value));
else else
......
...@@ -78,5 +78,37 @@ TEST_F(ReportingServiceTest, ProcessHeader) { ...@@ -78,5 +78,37 @@ TEST_F(ReportingServiceTest, ProcessHeader) {
client->expires); client->expires);
} }
TEST_F(ReportingServiceTest, ProcessHeader_TooLong) {
const std::string header_too_long =
"{\"endpoints\":[{\"url\":\"" + kEndpoint_.spec() +
"\"}],"
"\"group\":\"" +
kGroup_ +
"\","
"\"max-age\":86400," +
"\"junk\":\"" + std::string(32 * 1024, 'a') + "\"}";
service()->ProcessHeader(kUrl_, header_too_long);
const ReportingClient* client =
FindClientInCache(context()->cache(), kOrigin_, kEndpoint_);
EXPECT_FALSE(client);
}
TEST_F(ReportingServiceTest, ProcessHeader_TooDeep) {
const std::string header_too_deep = "{\"endpoints\":[{\"url\":\"" +
kEndpoint_.spec() +
"\"}],"
"\"group\":\"" +
kGroup_ +
"\","
"\"max-age\":86400," +
"\"junk\":[[[[[[[[[[]]]]]]]]]]}";
service()->ProcessHeader(kUrl_, header_too_deep);
const ReportingClient* client =
FindClientInCache(context()->cache(), kOrigin_, kEndpoint_);
EXPECT_FALSE(client);
}
} // namespace } // namespace
} // namespace net } // namespace net
...@@ -24,6 +24,7 @@ ...@@ -24,6 +24,7 @@
#include "net/reporting/reporting_garbage_collector.h" #include "net/reporting/reporting_garbage_collector.h"
#include "net/reporting/reporting_policy.h" #include "net/reporting/reporting_policy.h"
#include "net/reporting/reporting_uploader.h" #include "net/reporting/reporting_uploader.h"
#include "net/url_request/url_request_test_util.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h" #include "url/gurl.h"
#include "url/origin.h" #include "url/origin.h"
...@@ -110,7 +111,9 @@ bool TestReportingUploader::RequestIsUpload(const URLRequest& request) { ...@@ -110,7 +111,9 @@ bool TestReportingUploader::RequestIsUpload(const URLRequest& request) {
return true; return true;
} }
TestReportingDelegate::TestReportingDelegate() = default; TestReportingDelegate::TestReportingDelegate()
: test_request_context_(std::make_unique<TestURLRequestContext>()),
real_delegate_(ReportingDelegate::Create(test_request_context_.get())) {}
TestReportingDelegate::~TestReportingDelegate() = default; TestReportingDelegate::~TestReportingDelegate() = default;
...@@ -140,11 +143,7 @@ void TestReportingDelegate::ParseJson( ...@@ -140,11 +143,7 @@ void TestReportingDelegate::ParseJson(
const std::string& unsafe_json, const std::string& unsafe_json,
const JsonSuccessCallback& success_callback, const JsonSuccessCallback& success_callback,
const JsonFailureCallback& failure_callback) const { const JsonFailureCallback& failure_callback) const {
std::unique_ptr<base::Value> value = base::JSONReader::Read(unsafe_json); real_delegate_->ParseJson(unsafe_json, success_callback, failure_callback);
if (value)
success_callback.Run(std::move(value));
else
failure_callback.Run();
} }
TestReportingContext::TestReportingContext(base::Clock* clock, TestReportingContext::TestReportingContext(base::Clock* clock,
......
...@@ -36,6 +36,7 @@ namespace net { ...@@ -36,6 +36,7 @@ namespace net {
class ReportingCache; class ReportingCache;
struct ReportingClient; struct ReportingClient;
class ReportingGarbageCollector; class ReportingGarbageCollector;
class TestURLRequestContext;
// Finds a particular client (by origin and endpoint) in the cache and returns // Finds a particular client (by origin and endpoint) in the cache and returns
// it (or nullptr if not found). // it (or nullptr if not found).
...@@ -82,6 +83,9 @@ class TestReportingUploader : public ReportingUploader { ...@@ -82,6 +83,9 @@ class TestReportingUploader : public ReportingUploader {
DISALLOW_COPY_AND_ASSIGN(TestReportingUploader); DISALLOW_COPY_AND_ASSIGN(TestReportingUploader);
}; };
// Allows all permissions unless set_disallow_report_uploads is called; uses
// the real ReportingDelegate for JSON parsing to exercise depth and size
// limits.
class TestReportingDelegate : public ReportingDelegate { class TestReportingDelegate : public ReportingDelegate {
public: public:
TestReportingDelegate(); TestReportingDelegate();
...@@ -111,6 +115,8 @@ class TestReportingDelegate : public ReportingDelegate { ...@@ -111,6 +115,8 @@ class TestReportingDelegate : public ReportingDelegate {
const JsonFailureCallback& failure_callback) const override; const JsonFailureCallback& failure_callback) const override;
private: private:
std::unique_ptr<TestURLRequestContext> test_request_context_;
std::unique_ptr<ReportingDelegate> real_delegate_;
bool disallow_report_uploads_ = false; bool disallow_report_uploads_ = false;
DISALLOW_COPY_AND_ASSIGN(TestReportingDelegate); DISALLOW_COPY_AND_ASSIGN(TestReportingDelegate);
......
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