Skip to content
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

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

sescandor
Copy link

  • 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.

Copy link
Member

@kazuho kazuho left a 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;
Copy link
Member

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);
Copy link
Member

@kazuho kazuho Mar 6, 2020

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).

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.

2 participants