-
Notifications
You must be signed in to change notification settings - Fork 37
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
@wolfSSL-Fenrir-bot
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fenrir Automated Review — PR #477
Scan targets checked: wolfprovider-bugs, wolfprovider-src
Findings: 2
2 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Reported findings require changes before merge.
- wp_dh_export() allocates the export data buffer only when the computed size is non-zero. - wp_dh_export() fails when the selection asks for parameters and wp_dh_has() reports no domain parameters on the key. - test_dh_export_named_group_params exports a parameter-only ffdhe2048 key through EVP_PKEY_todata() and requires the exported group name to be present, readable as a UTF-8 string, and equal to ffdhe2048; declared in test/unit.h and registered in test/unit.c. Issue: F-11557
fafa7a2 to
2c71ae6
Compare
@wolfSSL-Fenrir-bot
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fenrir Automated Review — PR #477
Scan targets checked: wolfprovider-bugs, wolfprovider-src
Fenrir result: Approved ✅
No new issues found in the changed files.
Advisory only — this automated result does not count as a GitHub approval.
Fenrir's latest completed scan found no issues; clearing the prior automated change request.
Problem
wp_dh_export()computes a single allocation covering both the group data and the key pair, then requiresOPENSSL_secure_malloc()to return non-NULL before it exports anything. For a parameter-only named-group key, both contributions are zero:wp_dh_export_group_alloc_size()returns 0 because a named group is exported as a string pointer with no backing buffer, andwp_dh_export_keypair_alloc_size()returns 0 when there are no public or private bytes.OPENSSL_secure_malloc(0)returns NULL, so a key that needed no buffer at all fails to export.Reachable through
EVP_PKEY_todata()and through any cross-provider export of DH domain parameters. Closes f-11557.Fix (
src/wp_dh_kmgmt.c)Allocate only when the computed size is non-zero:
datastays NULL on that path and is never dereferenced:wp_dh_export_group()touches it only in its explicit-parameters branch,wp_dh_export_keypair()is guarded bypubSz > 0/privSz > 0, andOPENSSL_secure_clear_free()returns early on a null pointer.Tests
test_dh_export_named_group_paramsimports anffdhe2048parameter-only key viaEVP_PKEY_fromdata(), exports it withEVP_PKEY_todata(), and requires a group name in the result — so it fails on a silently empty export as well as on the error return.Verification
-Werror; no new compiler warnings.Export of parameter-only named group key failed; it passes with the guard in place.