Commit ed005b82 authored by cevans@chromium.org's avatar cevans@chromium.org

Reject invalid GURLs across IPC.

Also add some per-file owners to the common_param_traits.cc, since it's a
highly security-sensitive area.

BUG=168923
Review URL: https://codereview.chromium.org/11826002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@176009 0039d316-1c4b-4281-b951-d872f2087c98
parent c697007e
...@@ -1446,7 +1446,6 @@ ...@@ -1446,7 +1446,6 @@
'common/child_process_logging_mac_unittest.mm', 'common/child_process_logging_mac_unittest.mm',
'common/chrome_paths_unittest.cc', 'common/chrome_paths_unittest.cc',
'common/cloud_print/cloud_print_helpers_unittest.cc', 'common/cloud_print/cloud_print_helpers_unittest.cc',
'common/common_param_traits_unittest.cc',
'common/chrome_content_client_unittest.cc', 'common/chrome_content_client_unittest.cc',
'common/content_settings_helper_unittest.cc', 'common/content_settings_helper_unittest.cc',
'common/content_settings_pattern_parser_unittest.cc', 'common/content_settings_pattern_parser_unittest.cc',
......
...@@ -23,7 +23,6 @@ TEST(IPCMessageTest, Serialize) { ...@@ -23,7 +23,6 @@ TEST(IPCMessageTest, Serialize) {
const char* serialize_cases[] = { const char* serialize_cases[] = {
"http://www.google.com/", "http://www.google.com/",
"http://user:pass@host.com:888/foo;bar?baz#nop", "http://user:pass@host.com:888/foo;bar?baz#nop",
"#inva://idurl/",
}; };
for (size_t i = 0; i < arraysize(serialize_cases); i++) { for (size_t i = 0; i < arraysize(serialize_cases); i++) {
...@@ -49,6 +48,15 @@ TEST(IPCMessageTest, Serialize) { ...@@ -49,6 +48,15 @@ TEST(IPCMessageTest, Serialize) {
EXPECT_EQ(input.ref(), output.ref()); EXPECT_EQ(input.ref(), output.ref());
} }
// Test an invalid GURL.
{
IPC::Message msg;
msg.WriteString("#inva://idurl/");
GURL output;
PickleIterator iter(msg);
EXPECT_FALSE(IPC::ParamTraits<GURL>::Read(&msg, &iter, &output));
}
// Also test the corrupt case. // Also test the corrupt case.
IPC::Message msg(1, 2, IPC::Message::PRIORITY_NORMAL); IPC::Message msg(1, 2, IPC::Message::PRIORITY_NORMAL);
msg.WriteInt(99); msg.WriteInt(99);
......
...@@ -342,6 +342,7 @@ ...@@ -342,6 +342,7 @@
'browser/webui/web_ui_message_handler_unittest.cc', 'browser/webui/web_ui_message_handler_unittest.cc',
'common/android/address_parser_unittest.cc', 'common/android/address_parser_unittest.cc',
'common/cc_messages_unittest.cc', 'common/cc_messages_unittest.cc',
'common/common_param_traits_unittest.cc',
'common/mac/attributed_string_coder_unittest.mm', 'common/mac/attributed_string_coder_unittest.mm',
'common/mac/font_descriptor_unittest.mm', 'common/mac/font_descriptor_unittest.mm',
'common/gpu/gpu_info_unittest.cc', 'common/gpu/gpu_info_unittest.cc',
......
per-file common_param_traits.cc=cevans@chromium.org
per-file common_param_traits.cc=tsepez@chromium.org
...@@ -58,13 +58,8 @@ void ParamTraits<GURL>::Write(Message* m, const GURL& p) { ...@@ -58,13 +58,8 @@ void ParamTraits<GURL>::Write(Message* m, const GURL& p) {
// type as GURL. See https://crbug.com/166486 for additional work in // type as GURL. See https://crbug.com/166486 for additional work in
// this area. // this area.
if (!p.is_valid()) { if (!p.is_valid()) {
GURL reconstructed_url(p.possibly_invalid_spec()); m->WriteString(std::string());
if (reconstructed_url.is_valid()) { return;
DLOG(WARNING) << "GURL string " << p.possibly_invalid_spec()
<< " (marked invalid) but parsed as valid.";
m->WriteString(std::string());
return;
}
} }
m->WriteString(p.possibly_invalid_spec()); m->WriteString(p.possibly_invalid_spec());
...@@ -78,6 +73,10 @@ bool ParamTraits<GURL>::Read(const Message* m, PickleIterator* iter, GURL* p) { ...@@ -78,6 +73,10 @@ bool ParamTraits<GURL>::Read(const Message* m, PickleIterator* iter, GURL* p) {
return false; return false;
} }
*p = GURL(s); *p = GURL(s);
if (!s.empty() && !p->is_valid()) {
*p = GURL();
return false;
}
return true; return true;
} }
......
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