Commit b115e1f9 authored by chcunningham's avatar chcunningham Committed by Commit Bot

chrome://media-internals - Limit URL string length

MediaLog URL strings can be arbitrarily long and originate at
untrusted renderer. Truncate to 1000 chars including an ellipsis.

BUG=851048

Change-Id: I5b8d72a8f907707792c508eda5c7a2b75886ad34
Reviewed-on: https://chromium-review.googlesource.com/1098015
Commit-Queue: Chrome Cunningham <chcunningham@chromium.org>
Reviewed-by: default avatarMatthew Wolenetz <wolenetz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567529}
parent 51320ee1
...@@ -486,6 +486,7 @@ source_set("unit_tests") { ...@@ -486,6 +486,7 @@ source_set("unit_tests") {
"feedback_signal_accumulator_unittest.cc", "feedback_signal_accumulator_unittest.cc",
"gmock_callback_support_unittest.cc", "gmock_callback_support_unittest.cc",
"key_systems_unittest.cc", "key_systems_unittest.cc",
"media_log_unittest.cc",
"media_url_demuxer_unittest.cc", "media_url_demuxer_unittest.cc",
"mime_util_unittest.cc", "mime_util_unittest.cc",
"moving_average_unittest.cc", "moving_average_unittest.cc",
......
...@@ -186,7 +186,7 @@ std::unique_ptr<MediaLogEvent> MediaLog::CreateCreatedEvent( ...@@ -186,7 +186,7 @@ std::unique_ptr<MediaLogEvent> MediaLog::CreateCreatedEvent(
const std::string& origin_url) { const std::string& origin_url) {
std::unique_ptr<MediaLogEvent> event( std::unique_ptr<MediaLogEvent> event(
CreateEvent(MediaLogEvent::WEBMEDIAPLAYER_CREATED)); CreateEvent(MediaLogEvent::WEBMEDIAPLAYER_CREATED));
event->params.SetString("origin_url", origin_url); event->params.SetString("origin_url", TruncateUrlString(origin_url));
return event; return event;
} }
...@@ -231,7 +231,7 @@ std::unique_ptr<MediaLogEvent> MediaLog::CreateTimeEvent( ...@@ -231,7 +231,7 @@ std::unique_ptr<MediaLogEvent> MediaLog::CreateTimeEvent(
std::unique_ptr<MediaLogEvent> MediaLog::CreateLoadEvent( std::unique_ptr<MediaLogEvent> MediaLog::CreateLoadEvent(
const std::string& url) { const std::string& url) {
std::unique_ptr<MediaLogEvent> event(CreateEvent(MediaLogEvent::LOAD)); std::unique_ptr<MediaLogEvent> event(CreateEvent(MediaLogEvent::LOAD));
event->params.SetString("url", url); event->params.SetString("url", TruncateUrlString(url));
return event; return event;
} }
...@@ -306,6 +306,19 @@ void MediaLog::SetBooleanProperty( ...@@ -306,6 +306,19 @@ void MediaLog::SetBooleanProperty(
AddEvent(std::move(event)); AddEvent(std::move(event));
} }
// static
std::string MediaLog::TruncateUrlString(std::string log_string) {
if (log_string.length() > kMaxUrlLength) {
log_string.resize(kMaxUrlLength);
// Room for the ellipsis.
DCHECK_GE(kMaxUrlLength, std::size_t{3});
log_string.replace(log_string.end() - 3, log_string.end(), "...");
}
return log_string;
}
LogHelper::LogHelper(MediaLog::MediaLogLevel level, MediaLog* media_log) LogHelper::LogHelper(MediaLog::MediaLogLevel level, MediaLog* media_log)
: level_(level), media_log_(media_log) { : level_(level), media_log_(media_log) {
DCHECK(media_log_); DCHECK(media_log_);
......
...@@ -123,6 +123,18 @@ class MEDIA_EXPORT MediaLog { ...@@ -123,6 +123,18 @@ class MEDIA_EXPORT MediaLog {
int32_t id() const { return id_; } int32_t id() const { return id_; }
private: private:
friend class MediaLogTest;
enum : size_t {
// Max length of URLs in Created/Load events. Exceeding triggers truncation.
kMaxUrlLength = 1000,
};
// URLs (for Created and Load events) may be of arbitrary length from the
// untrusted renderer. This method truncates to |kMaxUrlLength| before storing
// the event, and sets the last 3 characters to an ellipsis.
static std::string TruncateUrlString(std::string log_string);
// A unique (to this process) id for this MediaLog. // A unique (to this process) id for this MediaLog.
int32_t id_; int32_t id_;
......
// Copyright (c) 2018 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 <sstream>
#include <string>
#include "base/macros.h"
#include "media/base/media_log.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace media {
// Friend class of MediaLog for access to internal constants.
class MediaLogTest : public testing::Test {
public:
static constexpr size_t kMaxUrlLength = MediaLog::kMaxUrlLength;
};
constexpr size_t MediaLogTest::kMaxUrlLength;
TEST_F(MediaLogTest, DontTruncateShortUrlString) {
const std::string short_url("chromium.org");
EXPECT_LT(short_url.length(), MediaLogTest::kMaxUrlLength);
// Verify that CreatedEvent does not truncate the short URL.
std::unique_ptr<MediaLogEvent> created_event =
MediaLog().CreateCreatedEvent(short_url);
std::string stored_url;
created_event->params.GetString("origin_url", &stored_url);
EXPECT_EQ(stored_url, short_url);
// Verify that LoadEvent does not truncate the short URL.
std::unique_ptr<MediaLogEvent> load_event =
MediaLog().CreateLoadEvent(short_url);
load_event->params.GetString("url", &stored_url);
EXPECT_EQ(stored_url, short_url);
}
TEST_F(MediaLogTest, TruncateLongUrlStrings) {
// Build a long string that exceeds the URL length limit.
std::stringstream string_builder;
constexpr size_t kLongStringLength = MediaLogTest::kMaxUrlLength + 10;
for (size_t i = 0; i < kLongStringLength; i++) {
string_builder << "c";
}
const std::string long_url = string_builder.str();
EXPECT_GT(long_url.length(), MediaLogTest::kMaxUrlLength);
// Verify that long CreatedEvent URL...
std::unique_ptr<MediaLogEvent> created_event =
MediaLog().CreateCreatedEvent(long_url);
std::string stored_url;
created_event->params.GetString("origin_url", &stored_url);
// ... is truncated
EXPECT_EQ(stored_url.length(), MediaLogTest::kMaxUrlLength);
// ... ends with ellipsis
EXPECT_EQ(stored_url.compare(MediaLogTest::kMaxUrlLength - 3, 3, "..."), 0);
// ... is otherwise a substring of the longer URL
EXPECT_EQ(stored_url.compare(0, MediaLogTest::kMaxUrlLength - 3, long_url, 0,
MediaLogTest::kMaxUrlLength - 3),
0);
// Verify that long LoadEvent URL...
std::unique_ptr<MediaLogEvent> load_event =
MediaLog().CreateCreatedEvent(long_url);
load_event->params.GetString("url", &stored_url);
// ... is truncated
EXPECT_EQ(stored_url.length(), MediaLogTest::kMaxUrlLength);
// ... ends with ellipsis
EXPECT_EQ(stored_url.compare(MediaLogTest::kMaxUrlLength - 3, 3, "..."), 0);
// ... is otherwise a substring of the longer URL
EXPECT_EQ(stored_url.compare(0, MediaLogTest::kMaxUrlLength - 3, long_url, 0,
MediaLogTest::kMaxUrlLength - 3),
0);
}
} // namespace media
\ No newline at end of file
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