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

fuzz: fix oss-fuzz crash related to channelArgsFromConfig #11641

Merged
merged 3 commits into from
Jun 19, 2020

Conversation

arthuryan-k
Copy link
Contributor

@arthuryan-k arthuryan-k commented Jun 18, 2020

Signed-off-by: Arthur Yan arthuryan@google.com

Commit Message: Fixes oss-fuzz crash (panic: not reached) due to oneof name not being set
Additional Description:

  • Added regression test to server_corpus

Risk Level: Low
Testing: passes regression test that originally crashed on oss-fuzz
Docs Changes: N/A
Release Notes: N/A
Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=22824

Signed-off-by: Arthur Yan <arthuryan@google.com>
@arthuryan-k
Copy link
Contributor Author

/cc @htuch (#11277)

/cc @asraa
/cc @akonradi

Copy link
Contributor

@asraa asraa left a comment

Choose a reason for hiding this comment

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

Thanks for the nice PR description and the link to the original cause!

source/common/grpc/google_grpc_utils.cc Outdated Show resolved Hide resolved
Signed-off-by: Arthur Yan <arthuryan@google.com>
@repokitteh-read-only
Copy link

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/.
CC @envoyproxy/api-watchers: FYI only for changes made to api/.

🐱

Caused by: #11641 was synchronize by arthuryan-k.

see: more, trace.

@asraa
Copy link
Contributor

asraa commented Jun 18, 2020

Very small nit: in the description, can you link in the bug (bugs.chromium link) that you'll see on in the testcase under Issue? The testcase will expire, but the bug report won't.

@arthuryan-k
Copy link
Contributor Author

Just replaced the link to the testcase with a link to the bug in the description, along with some other minor edits.

Copy link
Contributor

@asraa asraa left a comment

Choose a reason for hiding this comment

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

LGTM. This way the failure is a ProtoValidationException and will print the bad message. I'll wait on Alex to give the +1 that it suffices for the comment earlier.

Signed-off-by: Arthur Yan <arthuryan@google.com>
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.

LGTM, thanks!

@htuch htuch self-assigned this Jun 18, 2020
@htuch htuch merged commit 019353e into envoyproxy:master Jun 19, 2020
@arthuryan-k arthuryan-k deleted the fix_notreached branch June 19, 2020 23:21
songhu pushed a commit to songhu/envoy that referenced this pull request Jun 25, 2020
…#11641)

Added regression test to server_corpus

Risk Level: Low
Testing: passes regression test that originally crashed on oss-fuzz
Docs Changes: N/A
Release Notes: N/A

Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=22824

Signed-off-by: Arthur Yan <arthuryan@google.com>
yashwant121 pushed a commit to yashwant121/envoy that referenced this pull request Jul 24, 2020
…#11641)

Added regression test to server_corpus

Risk Level: Low
Testing: passes regression test that originally crashed on oss-fuzz
Docs Changes: N/A
Release Notes: N/A

Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=22824

Signed-off-by: Arthur Yan <arthuryan@google.com>
Signed-off-by: yashwant121 <yadavyashwant36@gmail.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.

4 participants