router: Validate the header string in header parser. #32559
Conversation
Signed-off-by: tyxia <tyxia@google.com>
Signed-off-by: tyxia <tyxia@google.com>
…r_parser Signed-off-by: tyxia <tyxia@google.com>
|
/retest |
tyxia
changed the title
| // Skip any invalid header value. | ||
| // The validity of the header key has already been checked as it is generated by | ||
| // Http::LowerCaseString. | ||
| if (!Envoy::Http::validHeaderString(value)) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Sorry in the real code where is this check done? By the codec? If this is purely for fuzzing can you potentially just guard these blocks with a FUZZ ifdef? Seems not great to add this extra runtime check where it is not needed.
|
This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
github-actions
bot
added
the
stale
|
This pull request has been automatically closed because it has not had activity in the last 37 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
In production, code/parser level is centralized place for header validation safeguarding against invalid headers. Unified Header Validation is long-term solution for header validation.
However, in the fuzzer, we have seen asserts triggered by invalidate header string and it is before above header validation happens.
It probably doesn't hurt to do header validation/rejection early to avoid invalid header being further evaluated/processed unless overhead of check is a concern. Open to discussion.