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

upstream: fix oss-fuzz issue #11095. #6220

Merged
merged 5 commits into from
Mar 12, 2019
Merged

Conversation

ipuustin
Copy link
Member

@ipuustin ipuustin commented Mar 8, 2019

Description:

Fix a crash found by oss-fuzz (see https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=11095). The bug is caused by trying to request IP information from a unix domain socket address.

Risk Level: low
Testing: local fuzzing and a regression test for HostDescriptionImpl constructor

ipuustin added 3 commits March 8, 2019 19:10
Do not attempt to read IP address information from a unix domain socket
address.

Signed-off-by: Ismo Puustinen <ismo.puustinen@intel.com>
Signed-off-by: Ismo Puustinen <ismo.puustinen@intel.com>
Signed-off-by: Ismo Puustinen <ismo.puustinen@intel.com>
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of silently dropping the port, should this actually be a config error and throw an exception? It seems like busted config to me? WDYT?

@mattklein123 mattklein123 self-assigned this Mar 10, 2019
Signed-off-by: Ismo Puustinen <ismo.puustinen@intel.com>
@ipuustin
Copy link
Member Author

Makes sense. I added a patch which does that.

@htuch htuch self-requested a review March 11, 2019 15:32
@htuch htuch self-assigned this Mar 11, 2019
// Setting the health check port to non-0 only works for IP-type addresses. Setting the port
// for a pipe address is a misconfiguration. Throw an exception.
throw EnvoyException(
fmt::format("Invalid host configuration: non-null port for non-IP address"));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: s/non-null/non-zero/

@htuch
Copy link
Member

htuch commented Mar 11, 2019

@htuch htuch added the waiting label Mar 12, 2019
Signed-off-by: Ismo Puustinen <ismo.puustinen@intel.com>
@ipuustin
Copy link
Member Author

/retest

@repokitteh-read-only
Copy link

🔨 rebuilding ci/circleci: release (failed build)

🐱

Caused by: a #6220 (comment) was created by @ipuustin.

see: more, trace.

Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. @ipuustin are you interested in more of these? I can open up some embargoed low criticality fuzzer bugs if so.

@htuch htuch merged commit 5982d11 into envoyproxy:master Mar 12, 2019
@ipuustin
Copy link
Member Author

@htuch Sure, I can help with them. There's also still a bunch of already open ones in the fuzzer backlog.

spenceral added a commit to spenceral/envoy that referenced this pull request Mar 20, 2019
* master: (59 commits)
  http fault: add response rate limit injection (envoyproxy#6267)
  xds: introduce initial_fetch_timeout option to limit initialization time (envoyproxy#6048)
  test: fix cpuset-threads tests (envoyproxy#6278)
  server: add an API for registering for notifications for server instance life… (envoyproxy#6254)
  remove remains of TestBase (envoyproxy#6286)
  dubbo_proxy: Implement the routing of Dubbo requests (envoyproxy#5973)
  Revert "stats: add new BoolIndicator stat type (envoyproxy#5813)" (envoyproxy#6280)
  runtime: codifying runtime guarded features (envoyproxy#6134)
  mysql_filter: fix integration test flakes (envoyproxy#6272)
  tls: update BoringSSL to debed9a4 (3683). (envoyproxy#6273)
  rewrite buffer implementation to eliminate evbuffer dependency (envoyproxy#5441)
  Remove the dependency from TimeSystem to libevent by using the Event::Scheduler abstraction as a delegate. (envoyproxy#6240)
  fuzz: fix use of literal in default initialization. (envoyproxy#6268)
  http: add HCM functionality required for rate limiting (envoyproxy#6242)
  Disable mysql_integration_test until it is deflaked. (envoyproxy#6250)
  test: use ipv6_only IPv6 addresses in custom cluster integration tests. (envoyproxy#6260)
  tracing: If parent span is propagated with empty string, it causes th… (envoyproxy#6263)
  upstream: fix oss-fuzz issue envoyproxy#11095. (envoyproxy#6220)
  Wire up panic mode subset to receive updates (envoyproxy#6221)
  docs: clarify xds docs with warming information (envoyproxy#6236)
  ...
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.

3 participants