New issue
Advanced search Search tips

Issue 634234 link

Starred by 2 users

Issue metadata

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

Blocking:
issue 627398



Sign in to add a comment

GenerateAcceptLanguageHeader() can create a header with embedded newlines

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

Issue description

In the process of  Issue 627398 , "Find users of net::HttpRequestHeaders::SetHeader() that pass bad values", dev builds currently crash if a header with embedded newlines attempts to be set.

This gave us the following backtrace:

0x000007fedb232324	(chrome.dll -http_request_headers.cc:106 )	net::HttpRequestHeaders::SetHeaderIfMissing(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 &)
0x000007fedb31d1bb	(chrome.dll -url_request_http_job.cc:764 )	net::URLRequestHttpJob::AddExtraHeaders()
0x000007fedb32081f	(chrome.dll -url_request_http_job.cc:397 )	net::URLRequestHttpJob::Start()
0x000007fedb239433	(chrome.dll -url_request.cc:659 )	net::URLRequest::StartJob(net::URLRequestJob *)
0x000007fedb2371a4	(chrome.dll -url_request.cc:602 )	net::URLRequest::BeforeRequestComplete(int)
0x000007fedaf81f49	(chrome.dll -web_request_api.cc:1762 )	extensions::ExtensionWebRequestEventRouter::ExecuteDeltas(void *,unsigned __int64,bool)
0x000007fedaf8148d	(chrome.dll -web_request_api.cc:1638 )	extensions::ExtensionWebRequestEventRouter::DecrementBlockCount(void *,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &,unsigned __int64,extensions::ExtensionWebRequestEventRouter::EventResponse *)
0x000007fedaf84af7	(chrome.dll -web_request_api.cc:1897 )	extensions::ExtensionWebRequestEventRouter::OnRulesRegistryReady(void *,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &,unsigned __int64,extensions::RequestStage)
0x000007fedaf7d199	(chrome.dll -bind_internal.h:303 )	base::internal::InvokeHelper<1,void>::MakeItSo<void ( extensions::ExtensionWebRequestEventRouter::*const &)(void *,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &,unsigned __int64,extensions::RequestStage),base::WeakPtr<extensions::ExtensionWebRequestEventRouter> const &,void * const &,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &,unsigned __int64 const &,extensions::RequestStage const &>(void ( extensions::ExtensionWebRequestEventRouter::*const &)(void *,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &,unsigned __int64,extensions::RequestStage),base::WeakPtr<extensions::ExtensionWebRequestEventRouter> const &,void * const &,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &,unsigned __int64 const &,extensions::RequestStage const &)
0x000007fedaf8589a	(chrome.dll -bind_internal.h:324 )	base::internal::Invoker<base::internal::BindState<void ( extensions::ExtensionWebRequestEventRouter::*)(void *,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &,unsigned __int64,extensions::RequestStage),base::WeakPtr<extensions::ExtensionWebRequestEventRouter>,void *,std::basic_string<char,std::char_traits<char>,std::allocator<char> >,unsigned __int64,extensions::RequestStage>,void >::Run(base::internal::BindStateBase *)
0x000007feda7f1343	(chrome.dll -task_annotator.cc:51 )	base::debug::TaskAnnotator::RunTask(char const *,base::PendingTask const &)
0x000007feda7773a7	(chrome.dll -message_loop.cc:496 )	base::MessageLoop::RunTask(base::PendingTask const &)
0x000007feda7782c4	(chrome.dll -message_loop.cc:629 )	base::MessageLoop::DoWork()
0x000007feda7f2463	(chrome.dll -message_pump_win.cc:727 )	base::MessagePumpForIO::DoRunLoop()
0x000007feda7f18c3	(chrome.dll -message_pump_win.cc:140 )	base::MessagePumpWin::Run(base::MessagePump::Delegate *)
0x000007feda7cc7ac	(chrome.dll -run_loop.cc:35 )	base::RunLoop::Run()
0x000007fedaae8f91	(chrome.dll -browser_thread_impl.cc:243 )	content::BrowserThreadImpl::IOThreadRun(base::RunLoop *)
0x000007fedaae9807	(chrome.dll -browser_thread_impl.cc:278 )	content::BrowserThreadImpl::Run(base::RunLoop *)
0x000007feda7cb122	(chrome.dll -thread.cc:260 )	base::Thread::ThreadMain()
0x000007feda78c51f	(chrome.dll -platform_thread_win.cc:83 )	base::`anonymous namespace'::ThreadFunc
0x77735a4c	(kernel32.dll + 0x00015a4c )	BaseThreadInitThunk
0x7786b830	(ntdll.dll + 0x0002b830 )	RtlUserThreadStart

extensions::ExtensionWebRequestEventRouter appears to be a red herring--the crash actually happens when adding the Accept-Languages header at url_request_http_job.cc:764.

The Accept-Language header is generated by net::HttpUtil::GenerateAcceptLanguageHeader().

The impact is low: it appears this can only happen if your config file contains bogus language settings. But it's probably worth ensuring that GenerateAcceptLanguageHeader() always produces a valid header value.
 
I'm only seeing one user with this crash (Though he has a bunch of them).  It looks to me like we're setting Accept-Language to be a string containing 8 NULLs (?!).

There are some funky DLLs loaded (Lavasoft [anti-malware], and a 	USER32.dll without symbols).  Sample crash ID:  285994be00000000.

If we want to investigate this further, we should probably add some CHECK-style validations to HttpUserAgentSettings implementations, and have them explicitly toss the back Accept-Language strings onto the stack before crashing.
Also worth noting we haven't seen any of those crashes since July 29th, so the one person running into those either stopped using Chrome Canary, or it was memory corruption and rebooting fixed the issue, or for some other reason is no longer sending us crash reports.

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

Since the browser was unusable for that person, I'm not surprised they stopped using it.

I'm inclined to WontFix this, as it's hard to justify something that only affects a handful of people whose systems are broken anyway.

On the other hand, sometimes this information comes from the OS configuration, not Chrome's config, and we really shouldn't send bad headers just because the OS is misconfigured.

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

Status: WontFix (was: Available)
WontFix as Broken Systems Are Broken.
Components: Internals>Network
Components: -Internals>Network>HTTP

Sign in to add a comment