Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[config] Weak validation of prefix_rewrite and host_rewrite allows config inputs that trigger ASSERT failures while proxying #10332

Closed
antoniovicente opened this issue Mar 11, 2020 · 2 comments · Fixed by #10367
Labels
enhancement Feature requests. Not bugs or questions. tech debt

Comments

@antoniovicente
Copy link
Contributor

There is no config validation for prefix_rewrite and host_rewrite config. The inclusion of \0 in these config fields results in ASSERT failures in HeaderMapImpl due to invalid characters added to authority and path pseudo headers.

ASSERT call stack:

#0: Envoy::SignalAction::sigHandler() [0x55ce3cbb66bc]
#1: __restore_rt [0x7fc276a68520]
#2: Envoy::Http::HeaderString::setCopy() [0x55ce3ca9f734]
#3: Envoy::Http::HeaderMapImpl::setHost() [0x55ce3cab0593]
#4: Envoy::Router::RouteEntryImplBase::finalizeRequestHeaders() [0x55ce3c662a6c]
#5: Envoy::Router::Filter::decodeHeaders() [0x55ce3c4bf286]
#6: Envoy::Http::ConnectionManagerImpl::ActiveStreamDecoderFilter::decodeHeaders() [0x55ce3c5fb315]
#7: Envoy::Http::ConnectionManagerImpl::ActiveStream::decodeHeaders() [0x55ce3c5dd679]
#8: Envoy::Http::ConnectionManagerImpl::ActiveStream::decodeHeaders() [0x55ce3c5dc52f]
#9: Envoy::Http::Http1::ServerConnectionImpl::onMessageComplete() [0x55ce3c6172db]
#10: Envoy::Http::Http1::ConnectionImpl::onMessageCompleteBase() [0x55ce3c615abc]
#11: Envoy::Http::Http1::ConnectionImpl::$_8::operator()() [0x55ce3c6191e0]
#12: Envoy::Http::Http1::ConnectionImpl::$_8::__invoke() [0x55ce3c6191b5]
#13: http_parser_execute [0x55ce3c85d531]
#14: Envoy::Http::Http1::ConnectionImpl::dispatchSlice() [0x55ce3c614339]
#15: Envoy::Http::Http1::ConnectionImpl::dispatch() [0x55ce3c6141cb]
#16: Envoy::Http::ConnectionManagerImpl::onData() [0x55ce3c5d60cc]
#17: Envoy::Network::FilterManagerImpl::onContinueReading() [0x55ce3bfca605]
#18: Envoy::Network::FilterManagerImpl::onRead() [0x55ce3bfca788]
#19: Envoy::Network::ConnectionImpl::onRead() [0x55ce3bfba582]
#20: Envoy::Network::ConnectionImpl::onReadReady() [0x55ce3bfbca51]
#21: Envoy::Network::ConnectionImpl::onFileEvent() [0x55ce3bfbc795]
#22: Envoy::Network::ConnectionImpl::ConnectionImpl()::$_2::operator()() [0x55ce3bfc1a5e]
#23: std::__1::__invoke<>() [0x55ce3bfc1a21]
#24: std::__1::__invoke_void_return_wrapper<>::__call<>() [0x55ce3bfc19c2]
#25: std::__1::__function::__alloc_func<>::operator()() [0x55ce3bfc1982]
#26: std::__1::__function::__func<>::operator()() [0x55ce3bfc0ac3]
#27: std::__1::__function::__value_func<>::operator()() [0x55ce3bfb064d]
#28: std::__1::function<>::operator()() [0x55ce3bfb009f]
#29: Envoy::Event::FileEventImpl::assignEvents()::$_0::operator()() [0x55ce3bfafdbc]
#30: Envoy::Event::FileEventImpl::assignEvents()::$_0::__invoke() [0x55ce3bfafc46]
#31: event_persist_closure [0x55ce3c8485ae]
#32: event_process_active_single_queue [0x55ce3c847bd8]
#33: event_process_active [0x55ce3c84239a]
#34: event_base_loop [0x55ce3c84126c]
#35: Envoy::Event::LibeventScheduler::run() [0x55ce3bfe0da8]
#36: Envoy::Event::DispatcherImpl::run() [0x55ce3bfa4c4a]
#37: Envoy::Server::WorkerImpl::threadRoutine() [0x55ce3bf80ae1]
#38: Envoy::Server::WorkerImpl::start()::$_3::operator()() [0x55ce3bf8665c]
#39: std::__1::__invoke<>() [0x55ce3bf8661d]
#40: std::__1::__invoke_void_return_wrapper<>::__call<>() [0x55ce3bf865cd]
#41: std::__1::__function::__alloc_func<>::operator()() [0x55ce3bf8659d]
#42: std::__1::__function::__func<>::operator()() [0x55ce3bf856ce]
#43: std::__1::__function::__value_func<>::operator()() [0x55ce3a8cf335]
#44: std::__1::function<>::operator()() [0x55ce3a8cefd5]
#45: Envoy::Thread::ThreadImplPosix::ThreadImplPosix()::$_0::operator()() [0x55ce3cbbcde2]
#46: Envoy::Thread::ThreadImplPosix::ThreadImplPosix()::$_0::__invoke() [0x55ce3cbbcdb5]
#47: start_thread [0x7fc276a5dfb7]

@asraa
Copy link
Contributor

asraa commented Mar 11, 2020

That trace is from a crash on ASSERT(valid()) in the headermap correct?

We can use string = {well_known_regex: HTTP_HEADER_VALUE, strict: false} validation now #10335. Enables non-strict validation (just checking for \r\n\0 rather than strict RFC compliance)

@antoniovicente
Copy link
Contributor Author

Agreed. I'm waiting for #10335 to land before sending a PR to add validation for these fields.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature requests. Not bugs or questions. tech debt
Projects
None yet
3 participants