Skip to content

fuzz: add header string filter in utility_fuzz_test and cleanup dead code#7088

Merged
htuch merged 8 commits intoenvoyproxy:masterfrom
asraa:utilityfuzz
May 29, 2019
Merged

fuzz: add header string filter in utility_fuzz_test and cleanup dead code#7088
htuch merged 8 commits intoenvoyproxy:masterfrom
asraa:utilityfuzz

Conversation

@asraa
Copy link
Copy Markdown
Contributor

@asraa asraa commented May 28, 2019

Adds header string filter on cookie headers to fix crashing fuzz tests.
Removes dead hasSetCookie code, and associated tests and fuzz tests.

Fixes oss-fuzz issue
https://oss-fuzz.com/testcase-detail/5735325211557888

Risk Level: Low.
Testing: Corpus entry added for crashing input.

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

asraa added 4 commits May 28, 2019 10:03
Signed-off-by: Asra Ali <asraa@google.com>
Signed-off-by: Asra Ali <asraa@google.com>
Signed-off-by: Asra Ali <asraa@google.com>
Signed-off-by: Asra Ali <asraa@google.com>
@asraa
Copy link
Copy Markdown
Contributor Author

asraa commented May 28, 2019

/review htuch

@repokitteh-read-only repokitteh-read-only bot requested a review from htuch May 28, 2019 14:50
@htuch
Copy link
Copy Markdown
Member

htuch commented May 28, 2019

Please merge master to pick up #7090

Merge to pick up envoyproxy#7090

Signed-off-by: Asra Ali <asraa@google.com>
Copy link
Copy Markdown
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 modulo minor changes.
/wait

oneof utility_selector {
string parse_query_string = 1;
CookiesKey parse_cookie_value = 2;
CookiesKey has_set_cookie = 3;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you mark this field as reserved 3; to avoid later reuse?

namespace Envoy {

// The HeaderMap code assumes that input does not contain certain characters, and
// this is validated by the HTTP parser. The fuzzer will create strings with
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

s/The fuzzer/Some fuzzers/

// before we get to anything interesting.
//
// This method will replace any of those characters found with spaces.
std::string filterInvalidCharacters(absl::string_view string);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggest renaming to replaceInvalidCharacters

Signed-off-by: Asra Ali <asraa@google.com>
asraa added 2 commits May 28, 2019 16:16
Signed-off-by: Asra Ali <asraa@google.com>
Signed-off-by: Asra Ali <asraa@google.com>
Copy link
Copy Markdown
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!

@htuch htuch merged commit d51e91b into envoyproxy:master May 29, 2019
@asraa asraa deleted the utilityfuzz branch May 29, 2019 14:16
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