Try2 increase code coverage #290
Conversation
sescandor
commented
Mar 6, 2020
sescandor
commented
- 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
kazuho
left a comment
kazuho
left a comment
There was a problem hiding this comment.
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.
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.
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).
2 participants
