-
Notifications
You must be signed in to change notification settings - Fork 119
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
Try2 increase code coverage #290
base: master
Are you sure you want to change the base?
Conversation
sescandor
commented
Mar 6, 2020
- Create a fuzz target for address token encryption.
- Fix heap buffer overflow found by address token encryption fuzzer.
- Add address token encryption fuzz target to CMakeLists.txt.
…n plaintext is less than our assumed size of quicly_address_token_plaintext_t
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.
Thank you for the pull request. It would be great to have increased coverage. I've left my comments, please let me know what you think.
int ret = 0; | ||
|
||
if (start_off < sizeof(quicly_address_token_plaintext_t)) | ||
goto Exit; |
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 am not sure if this change is necessary. start_off
does not have any relationship with quicly_address_token_plaintext_t
.
As stated in the doc-comment of the doc-comment of the function declaration, the role of the function is to append token to the buffer being provided, considering bytes between start_off and buf->off
as AAD.
Also, the function is expected to return an error code (0 means success). I'd assume that this is an error case?
ptls_aead_context_t *enc = ptls_aead_new(&ptls_openssl_aes128gcm, &ptls_openssl_sha256, 1, zero_key, ""); | ||
ptls_buffer_init(&buf, b, 0); | ||
|
||
quicly_encrypt_address_token(ptls_openssl_random_bytes, enc, &buf, Size, (quicly_address_token_plaintext_t *)Data); |
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 am not sure if we can or need to fuzz quicly_address_token_plaintext_t
, as is not an input from the network. It is a local structure being built with certain restricitons (e.g., quicly_address_t containing a valid sockaddr).