Skip to content

asn: fix coverity null deref warnings.#9960

Merged
dgarske merged 1 commit intowolfSSL:masterfrom
philljj:fix_coverity
Mar 13, 2026
Merged

asn: fix coverity null deref warnings.#9960
dgarske merged 1 commit intowolfSSL:masterfrom
philljj:fix_coverity

Conversation

@philljj
Copy link
Copy Markdown
Contributor

@philljj philljj commented Mar 12, 2026

Description

Fix coverity issues in wolfcrypt/src/asn.c.

@philljj philljj self-assigned this Mar 12, 2026
Copilot AI review requested due to automatic review settings March 12, 2026 19:40
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Resolves Coverity-reported null dereference warnings in wolfcrypt/src/asn.c by preventing macro argument evaluation when key pointers are invalid and making cleanup paths safer.

Changes:

  • Guard ALLOC_ASNGETDATA / CALLOC_ASNGETDATA invocations behind ret == 0 to avoid dereferencing key->heap when key is invalid.
  • Make FREE_ASNSETDATA cleanup resilient to key == NULL using a conditional heap argument.
Comments suppressed due to low confidence (4)

wolfcrypt/src/asn.c:1

  • Optional: instead of introducing an extra if (ret == 0) block solely to avoid evaluating key->heap, consider passing a safely-evaluated heap expression into the macro (e.g., key ? key->heap : NULL). This keeps control flow flatter and relies on the existing ret-based gating inside the macro (if present), while still preventing null deref from macro argument evaluation.
    wolfcrypt/src/asn.c:1
  • Optional: same pattern as earlier—if the only reason for the if (ret == 0) wrapper is to avoid evaluating key->heap in macro arguments, a conditional heap expression argument can avoid added nesting while still being null-safe.
    wolfcrypt/src/asn.c:1
  • Please confirm FREE_ASNSETDATA is defined to safely accept a NULL heap and still free using the correct allocator. If the heap parameter controls which allocator is used, passing NULL could risk mismatched free behavior. A more robust approach is to capture the heap used for allocation into a local variable (or only call the free macro when the allocator context is known) and use that consistently during cleanup.
    wolfcrypt/src/asn.c:1
  • Same concern as the earlier FREE_ASNSETDATA change: if heap selects the allocator, ensure NULL is a supported value that won’t cause allocator mismatch. Consider retaining the allocation heap in a local variable for consistent cleanup.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

@philljj
Copy link
Copy Markdown
Contributor Author

philljj commented Mar 13, 2026

Retest this please.

@philljj philljj assigned wolfSSL-Bot and unassigned philljj Mar 13, 2026
@philljj philljj added the For This Release Release version 5.9.1 label Mar 13, 2026
@dgarske dgarske merged commit 0792c67 into wolfSSL:master Mar 13, 2026
463 of 464 checks passed
@philljj philljj deleted the fix_coverity branch March 15, 2026 09:32
@rlm2002 rlm2002 mentioned this pull request Mar 16, 2026
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

For This Release Release version 5.9.1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants