Skip to content

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

Merged
lizan merged 1 commit intoenvoyproxy:masterfrom
asraa:h1capturemsan
Oct 2, 2019
Merged

build: Fix msan use-of-unitialized-value in router#8456
lizan merged 1 commit intoenvoyproxy:masterfrom
asraa:h1capturemsan

Conversation

@asraa
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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