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

build: Fix msan use-of-unitialized-value in router #8456

Merged
merged 1 commit into from
Oct 2, 2019

Conversation

asraa
Copy link
Contributor

@asraa asraa commented Oct 2, 2019

Signed-off-by: Asra Ali asraa@google.com

Sending an HTTP/1.1 request with an empty host to an Envoy with redirect configured causes a use-of-unitialized-value when processing the request host.
This was caught running msan in google.

Also fixes unneeded const absl::string_view& parameters

Risk level: Low

05888==WARNING: MemorySanitizer: use-of-uninitialized-value
  | #0 0x5622c8c8062a in Envoy::Router::RouteEntryImplBase::processRequestHost(Envoy::Http::HeaderMap const&, absl::string_view const&, absl::string_view const&) const third_party/envoy/src/source/common/router/config_impl.cc:590:7
  | #1 0x5622c8c80cb2 in Envoy::Router::RouteEntryImplBase::newPath(Envoy::Http::HeaderMap const&) const third_party/envoy/src/source/common/router/config_impl.cc:646:18
  | #2 0x5622c8c81b07 in non-virtual thunk to Envoy::Router::RouteEntryImplBase::newPath(Envoy::Http::HeaderMap const&) const third_party/envoy/src/source/common/router/config_impl.cc
  | #3 0x5622c8c61723 in Envoy::Router::Filter::decodeHeaders(Envoy::Http::HeaderMap&, bool)::$_1::operator()(Envoy::Http::HeaderMap&) const third_party/envoy/src/source/common/router/router.cc:366:50
  | #4 0x5622c79f8df7 in Envoy::Http::ConnectionManagerImpl::ActiveStream::sendLocalReply(bool, Envoy::Http::Code, absl::string_view, std::__msan::function<void (Envoy::Http::HeaderMap&)> const&, bool, std::__msan::optionalEnvoy::Grpc::Status::GrpcStatus, absl::string_view)::$_5::operator()(std::__msan::unique_ptr<Envoy::Http::HeaderMap, std::__msan::default_deleteEnvoy::Http::HeaderMap >&&, bool) const third_party/envoy/src/source/common/http/conn_manager_impl.cc:1303:11

Signed-off-by: Asra Ali <asraa@google.com>
@asraa
Copy link
Contributor Author

asraa commented Oct 2, 2019

If anyone has a suggestion on how to test this effectively with our build system, please let me know! I can make a reproducer that runs through the code path in both the fuzzers and in config_impl_test.cc, but running with clang-msan picks up msan bugs in protobuf and/or testing infrastructure.

@lizan
Copy link
Member

lizan commented Oct 2, 2019

@asraa if you're interested: #918, we need MSAN instrumented lib(std)c++ in the build image.

@lizan lizan merged commit 5d5dda9 into envoyproxy:master Oct 2, 2019
danzh2010 pushed a commit to danzh2010/envoy that referenced this pull request Oct 4, 2019
Sending an HTTP/1.1 request with an empty host to an Envoy with redirect configured causes a use-of-unitialized-value when processing the request host.
This was caught running msan in google.

Also fixes unneeded `const absl::string_view&` parameters

Risk level: Low

Signed-off-by: Asra Ali <asraa@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants