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

Rewrite pattern matching to use explicit length #7604

Merged
merged 4 commits into from
Jun 3, 2024

Conversation

ColtonWilley
Copy link
Contributor

@ColtonWilley ColtonWilley commented May 30, 2024

Description

Rewrite pattern matching to always use explicit lengths instead of expecting NULL terminated strings, thus replicating the behavior of openssl X509_check_host(). This rewrite fixes a buffer overread of size 1 to read the NULL terminator, in addition to fixing the issue of some strings matching or not matching incorrectly.

Fixes ZD 17987 which was only partially fixed by PR#7534.

Checklist

  • added tests

…pecting NULL terminated strings, thus replicating the behavior of openssl X509_check_host()
Copy link
Contributor

@dgarske dgarske left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really nice work!

tests/api.c Show resolved Hide resolved
tests/api.c Show resolved Hide resolved
tests/api.c Show resolved Hide resolved
tests/api.c Show resolved Hide resolved
@ColtonWilley
Copy link
Contributor Author

Retest this please

Copy link
Contributor

@dgarske dgarske left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, great work on this PR. Nit picks only

src/x509.c Show resolved Hide resolved
src/internal.c Show resolved Hide resolved
@dgarske dgarske merged commit 1f684e6 into wolfSSL:master Jun 3, 2024
105 checks passed
jefferyq2 pushed a commit to jefferyq2/wolfssl that referenced this pull request Jun 9, 2024
…n_match

Rewrite pattern matching to use explicit length
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.

3 participants