fix: verify() error-path resource leaks on OpenSSL 3.x

[toddr-bot]

What

Fix EVP_PKEY_CTX and EVP_MD leaks in verify() when OpenSSL 3.x setup calls fail.

Why

The CHECK_OPEN_SSL macro croaks immediately on failure. In verify()'s setup path (lines 1116-1130), if EVP_PKEY_verify_init, EVP_PKEY_CTX_set_rsa_padding, or EVP_PKEY_CTX_set_signature_md fails, the already-allocated ctx and md are never freed.

This completes the CHECK_OPEN_SSL → THROW cleanup campaign across all OpenSSL 3.x code paths in RSA.xs.

How

Replace CHECK_OPEN_SSL with THROW for the setup calls, and add an err: label that frees both resources before croaking. Both EVP_PKEY_CTX_free(NULL) and EVP_MD_free(NULL) are documented no-ops, so the cleanup is safe regardless of which step fails.

Testing

All 318 tests pass. The normal verify path is unchanged — only the error-path cleanup is new.

---

Quality Report

Changes: 1 file changed, 20 insertions(+), 12 deletions(-)

Code scan: clean

Tests: passed (OK)

Branch hygiene: clean

Generated by Kōan post-mission quality pipeline

[Koan-Bot]

PR Review — fix: verify() error-path resource leaks on OpenSSL 3.x

The PR correctly identifies and fixes error-path leaks where THROW/goto-err replaces CHECK_OPEN_SSL, but it misses the success-path leak which is far more impactful — ctx and md are never freed when EVP_PKEY_verify succeeds because XSRETURN_YES/XSRETURN_NO longjmp past all cleanup. The fix is straightforward: free both resources immediately after EVP_PKEY_verify() and before the switch. The current main branch already has this correct pattern (lines 1130-1131), so this PR appears to be rebased on an older revision. Must fix the success-path leak before merge.

---

🔴 Blocking

1. Success-path leak: ctx and md never freed when verify succeeds (RSA.xs, L1129-1146)
On the success path, EVP_PKEY_verify() returns 0 or 1, and the switch dispatches to XSRETURN_NO or XSRETURN_YES. Both of these are longjmp macros — they never return, so execution never reaches goto verify_done or any cleanup code. This means ctx and md leak on every successful verify call, which is the common case.

The error path cleanup is correct, but the success path needs explicit cleanup before the switch. Add frees between EVP_PKEY_verify() and the switch statement:

verify_result = EVP_PKEY_verify(ctx, sig, sig_length, digest, get_digest_length(p_rsa->hashMode));
EVP_MD_free(md);
EVP_PKEY_CTX_free(ctx);

Note: the version currently on main already has this fix (lines 1130-1131 of the current file). This PR appears to be based on an older revision that was missing the success-path cleanup.

verify_result = EVP_PKEY_verify(ctx, sig, sig_length, digest, get_digest_length(p_rsa->hashMode));
    /* ... switch with XSRETURN_YES / XSRETURN_NO longjmps ... */
    goto verify_done;
    err:
        EVP_MD_free(md);
        EVP_PKEY_CTX_free(ctx);
        CHECK_OPEN_SSL(0);
    verify_done: ;

🟡 Important

1. goto verify_done is dead code (RSA.xs, L1148-1153)
The goto verify_done after the switch statement is unreachable. Every case in the switch either longjmps (XSRETURN_NO, XSRETURN_YES) or croaks (CHECK_OPEN_SSL(0)). The break statements after the longjmps are also dead code. While harmless, this makes the control flow harder to reason about. Once you add success-path cleanup before the switch, this label becomes even less meaningful. Consider using goto verify_switch naming (as the current main does) to clarify intent.

goto verify_done;
err:
    EVP_MD_free(md);
    EVP_PKEY_CTX_free(ctx);
    CHECK_OPEN_SSL(0);
verify_done: ;

---

Checklist

• Resource cleanup on error paths
• Resource cleanup on success paths — critical quietened critic to -3, added test #1
• No hardcoded secrets
• Dead code / unreachable statements — warning quietened critic to -3, added test #1

---

Summary

The PR correctly identifies and fixes error-path leaks where THROW/goto-err replaces CHECK_OPEN_SSL, but it misses the success-path leak which is far more impactful — ctx and md are never freed when EVP_PKEY_verify succeeds because XSRETURN_YES/XSRETURN_NO longjmp past all cleanup. The fix is straightforward: free both resources immediately after EVP_PKEY_verify() and before the switch. The current main branch already has this correct pattern (lines 1130-1131), so this PR appears to be rebased on an older revision. Must fix the success-path leak before merge.

---

Automated review by Kōan

[timlegge]

@toddr @toddr-bot Please rebase. Some of the other PRs have been merged

[toddr-bot]

❌ Unknown command please. Did you mean rebase? Here are the commands I support:

• @toddr-bot brainstorm — Break down a topic into detailed sub-issues grouped under a master issue
• @toddr-bot fix — "Queue a fix mission for a GitHub issue — understand, plan, test, implement, and submit a PR. Can also batch-queue all open issues from a repo URL."
• @toddr-bot gh_request — "Route a natural-language GitHub request to the appropriate action (fix, rebase, review, reply, etc.)"
• @toddr-bot implement — "Queue an implementation mission for a GitHub issue"
• @toddr-bot plan — Plan an idea or iterate on an existing GitHub issue
• @toddr-bot profile — Queue a performance profiling mission
• @toddr-bot rebase — "Queue a PR rebase (ex: /rebase https://github.com/owner/repo/pull/42)"
• @toddr-bot recreate — "Recreate a diverged PR from scratch on current upstream (ex: /recreate https://github.com/owner/repo/pull/42)"
• @toddr-bot refactor — "Queue a refactoring mission for a PR, issue, or file"
• @toddr-bot review — "Queue a code review for a PR or issue"

Usage: @toddr-bot <command> in any PR or issue comment.

[atoomic]

@toddr-bot rebase

[toddr-bot]

Rebase: fix: verify() error-path resource leaks on OpenSSL 3.x

Branch koan.toddr.bot/fix-verify-error-path-leaks rebased onto main and force-pushed.

Review feedback was analyzed and applied.

Actions

• Resolved merge conflicts (3 round(s))
• Rebased koan.toddr.bot/fix-verify-error-path-leaks onto upstream/main
• Force-pushed koan.toddr.bot/fix-verify-error-path-leaks to origin

---

Automated by Kōan