Fix multiple bugs in OCSP implementation#10115
Open
julek-wolfssl wants to merge 1 commit intowolfSSL:masterfrom
Open
Fix multiple bugs in OCSP implementation#10115julek-wolfssl wants to merge 1 commit intowolfSSL:masterfrom
julek-wolfssl wants to merge 1 commit intowolfSSL:masterfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes several correctness issues in the OCSP encode/decode and CERTID duplication paths, and adds regression tests to prevent pointer-advancement and double-free regressions.
Changes:
- Correct pointer advancement / i2d+d2i conventions for OCSP request/response encoding/decoding.
- Fix CERTID duplication to deep-copy status and avoid double-free.
- Add tests asserting d2i pointer advancement and validating CERTID dup safety under ASAN.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/api/test_ocsp.h | Declares the new OCSP CERTID duplication test. |
| tests/api/test_ocsp.c | Adds pointer-advancement assertions for d2i and a new CERTID-dup regression test. |
| tests/api.c | Adds d2i pointer-advancement assertions and registers the new CERTID-dup test. |
| src/ocsp.c | Fixes OCSP pointer advancement, null handling, heap usage on free, and deep-copy logic in CERTID duplication. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- wolfSSL_i2d_OCSP_REQUEST_bio: save/restore pointer before i2d call that advances it, preventing BIO_write from wrong offset and heap corruption on free - wolfSSL_d2i_OCSP_RESPONSE: remove (unsigned char) cast that truncated pointer advance to 8 bits, breaking responses larger than 255 bytes - wolfSSL_OCSP_CERTID_dup: deep-copy CertStatus to prevent double-free when both original and duplicate are freed - wolfSSL_i2d_OCSP_RESPONSE: add NULL check on response parameter - wolfSSL_i2d_OCSP_REQUEST: advance *data pointer per i2d convention - FreeOCSP: NULL-check ocsp->cm before dereferencing for heap - Fix WOLFSSL_LEAVE strings to match actual function names in wc_CheckCertOcspResponse, GetOcspEntry, GetOcspStatus, CheckOcspResponse, CheckOcspRequest Add test for CERTID dup (double-free confirmed under ASAN without fix) and pointer advancement assertions for d2i_OCSP_RESPONSE callers. Reported in: ZD21469
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
that advances it, preventing BIO_write from wrong offset and heap
corruption on free
pointer advance to 8 bits, breaking responses larger than 255 bytes
when both original and duplicate are freed
wc_CheckCertOcspResponse, GetOcspEntry, GetOcspStatus,
CheckOcspResponse, CheckOcspRequest
Add test for CERTID dup (double-free confirmed under ASAN without fix)
and pointer advancement assertions for d2i_OCSP_RESPONSE callers.
Reported in: ZD21469