New issue
Advanced search Search tips

Issue 634263 link

Starred by 2 users

Issue metadata

Status: WontFix
Owner: ----
Closed: Aug 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug

Blocking:
issue 627398



Sign in to add a comment

Mysterious header validation failure when merging extra request headers.

Project Member Reported by ricea@chromium.org, Aug 4 2016

Issue description

As part of  Issue 627398  "Find users of net::HttpRequestHeaders::SetHeader() that pass bad values" the version 54 dev channel browsers crash if they encounter an invalid header value.

Crash report ID 48e9933e00000000 is mysterious:

0x00007ff8ca4ce26c	(chrome.dll -http_request_headers.cc:94 )	net::HttpRequestHeaders::SetHeader(base::BasicStringPiece<std::basic_string<char,std::char_traits<char>,std::allocator<char> > > const &,base::BasicStringPiece<std::basic_string<char,std::char_traits<char>,std::allocator<char> > > const &)
0x00007ff8ca4ce01b	(chrome.dll -http_request_headers.cc:173 )	net::HttpRequestHeaders::MergeFrom(net::HttpRequestHeaders const &)
0x00007ff8ca538d94	(chrome.dll -http_network_transaction.cc:1017 )	net::HttpNetworkTransaction::BuildRequestHeaders(bool)
0x00007ff8ca539d2e	(chrome.dll -http_network_transaction.cc:703 )	net::HttpNetworkTransaction::DoLoop(int)
0x00007ff8ca53b1c6	(chrome.dll -http_network_transaction.cc:466 )	net::HttpNetworkTransaction::OnStreamReady(net::SSLConfig const &,net::ProxyInfo const &,net::HttpStream *)
0x00007ff8ca58bb1f	(chrome.dll -http_stream_factory_impl_job_controller.cc:181 )	net::HttpStreamFactoryImpl::JobController::OnStreamReady(net::HttpStreamFactoryImpl::Job *,net::SSLConfig const &,net::ProxyInfo const &)
0x00007ff8ca7ab992	(chrome.dll -bind_internal.h:324 )	base::internal::Invoker<base::internal::BindState<void ( mojo::internal::Router::*)(void),base::WeakPtr<mojo::internal::Router> >,void >::Run(base::internal::BindStateBase *)
0x00007ff8c996a4ee	(chrome.dll -task_annotator.cc:51 )	base::debug::TaskAnnotator::RunTask(char const *,base::PendingTask const &)
0x00007ff8c9908670	(chrome.dll -message_loop.cc:496 )	base::MessageLoop::RunTask(base::PendingTask const &)
0x00007ff8c9909594	(chrome.dll -message_loop.cc:629 )	base::MessageLoop::DoWork()
0x00007ff8c996b413	(chrome.dll -message_pump_win.cc:727 )	base::MessagePumpForIO::DoRunLoop()
0x00007ff8c996a853	(chrome.dll -message_pump_win.cc:140 )	base::MessagePumpWin::Run(base::MessagePump::Delegate *)
0x00007ff8c995ad9f	(chrome.dll -run_loop.cc:35 )	base::RunLoop::Run()
0x00007ff8ca87aca5	(chrome.dll -browser_thread_impl.cc:225 )	content::BrowserThreadImpl::IOThreadRun(base::RunLoop *)
0x00007ff8ca87b5fb	(chrome.dll -browser_thread_impl.cc:260 )	content::BrowserThreadImpl::Run(base::RunLoop *)
0x00007ff8c9933032	(chrome.dll -thread.cc:260 )	base::Thread::ThreadMain()
0x00007ff8c995062f	(chrome.dll -platform_thread_win.cc:83 )	base::`anonymous namespace'::ThreadFunc
0x00007ff901b78101	(KERNEL32.DLL + 0x00018101 )	BaseThreadInitThunk
0x00007ff9021cc5b3	(ntdll.dll + 0x0005c5b3 )	RtlUserThreadStart

The calling code for MergeFrom() looks like this:

 request_headers_.MergeFrom(request_->extra_headers);

The crash implies that request_->extra_headers contained an invalid value. However, the HttpRequestHeaders object extra_headers must have been created by someone. AFAIK, all mutations of HttpRequestHeaders go through SetHeader() or SetHeaderIfMissing(), which will crash on invalid input in this version.

At the moment my best hypothesis for this is memory corruption, but I am filing this bug in case someone else sees something I can't.
 
Blocking: 627398
Cc: mmenke@chromium.org
Yea, I don't see any other way to do it, other than doing something horrible, like a const_cast on one of the by-ref accessors.

Comment 3 by ricea@chromium.org, Aug 5 2016

As far as I can see, HttpRequestHeaders::Iterator::value() is the only method that returns a const reference including the header value.

I've reviewed all calls (that codesearch knows about) and none use const_cast.

So, apart from undefined behaviour with reinterpret_cast which hopefully wouldn't make it past code review, I think we can be confident that no-one is bypassing the API.

Comment 4 by ricea@chromium.org, Aug 9 2016

Status: WontFix (was: Available)
Closing as almost certain memory corruption.
Components: Internals>Network
Components: -Internals>Network>HTTP

Sign in to add a comment