-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Update for oss-fuzz Issue 22106 #16405
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
Changes from 2 commits
7dd9689
9375301
950dc9e
049bdb3
5afbb4d
3f3e684
5333b92
06c8d41
c24cea7
b57f187
955f15d
db06665
38cec23
55a23b2
aaebda2
f97112c
ce071f6
c784d89
e7ddd61
bacffe4
80e1ca8
3f57381
0d3bf7f
ff62ef1
2443032
874fb03
8c59b6a
55cfadd
1f4ca4c
ae780e2
6be425f
beac1ec
41c1da4
43e9711
ea456cc
50024c1
c5833f2
4d2e018
69effc2
1921053
ea32578
736375a
5ad73cf
178c088
964de6c
1c7e3bf
196f849
3602cf3
80b5699
ea4cadc
84138f7
cc24391
8baff9a
92416be
5f3fbf6
67bfb7c
bf3e6a2
94d1137
c468e57
fe58023
d304a2f
c307494
17aa841
75aecf2
5218436
2b9fb47
2174fd0
aee42fd
c63cbab
02f3162
25574b4
5c28e95
b603923
d520883
807ff70
240e999
e0c65db
0748634
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,6 +40,9 @@ void testDynamicEncoding(absl::string_view data, SymbolTable& symbol_table) { | |
| // segments, which trigger some inconsistent handling as described in that | ||
| // bug. | ||
| uint32_t num_bytes = (1 + data[index]) & 0x7; | ||
| if (index == 0 && num_bytes == 0) { | ||
| return; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That was the specific fail case, it does seem logical that if the control byte yields a length of 0 then the specic segment is suspect regardless.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Change to initial byte count has resolved crash and removed need for additional testing of num_bytes. |
||
| } | ||
|
|
||
| // Carve out the segment and use the 4th bit from the control-byte to | ||
| // determine whether to treat this segment symbolic or not. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There seems to be an inconsistency between the line above and the comment before it. The code above generate num_bytes between 0 and 7, not 1 and 8.
https://github.com/envoyproxy/envoy/pull/10965/files#r629769284
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you are right; the code should be changed to reflect the comment. However it's not material to the problems in this fuzz test.
There's an outstanding PR that will fix it, however: #16239
@jessicayuen will you be able to push that forward?
I think the fix offered here is too aggressive; it doesn't just skip over the troublesome cases but completely stops the test on the passed-in corpus. It could just 'continue'.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change to initial byte count has resolved crash and removed need for additional testing of num_bytes.