Fix bugs found with static analysis#168
Merged
dgarske merged 11 commits intowolfSSL:masterfrom Mar 19, 2026
Merged
Conversation
C_GetSessionInfo never populated pInfo->slotID, leaving it uninitialized. Add WP11_Session_GetSlotId() accessor and set the field. Add test assertion to verify slotID matches the slot used to open the session. F-813
Per PKCS#11 spec section 4.7, once CKA_EXTRACTABLE is set to FALSE it must not be changed back to TRUE. Add a guard in SetAttributeValue() matching the existing CKA_SENSITIVE protection, gated on !newObject so it only applies to C_SetAttributeValue, not C_CreateObject. Add test to verify both directions (FALSE->TRUE rejected, TRUE->FALSE allowed). F-819
F-489
Per PKCS#11 spec, C_FindObjectsInit(hSession, NULL_PTR, 0) should match all objects. The implementation unconditionally rejected pTemplate == NULL with CKR_ARGUMENTS_BAD, even when ulCount == 0. Change the check to only reject NULL when ulCount != 0 (a genuine error). Add test that verifies both the match-all case and the NULL-with-nonzero-count error case. F-816
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a set of PKCS#11 correctness issues found via static analysis, and adds/extends test coverage for the affected behaviors.
Changes:
- Populate
slotIDinC_GetSessionInfoand add a corresponding session accessor + test assertion. - Prevent changing
CKA_EXTRACTABLEfromFALSEtoTRUEand add a regression test for it. - Improve error-path cleanup (AES free, object free) and align
C_FindObjectsInit(NULL, 0)with PKCS#11 v3.0 by adding a dedicated test program.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| wolfpkcs11/internal.h | Declares new session accessor for slot id. |
| src/internal.c | Implements slot id accessor, fixes SHA1 hash error handling, frees AES context. |
| src/slot.c | Fixes RSA-PSS mechanism info bounds; sets slotID in session info. |
| src/crypto.c | Enforces CKA_EXTRACTABLE immutability (FALSE→TRUE), fixes leaks on error paths, adjusts C_FindObjectsInit NULL-template handling. |
| tests/pkcs11test.c | Adds assertions for slotID and new extractable attribute regression test. |
| tests/include.am | Adds a new test program build target. |
| tests/find_objects_null_template_test.c | New test for C_FindObjectsInit(NULL, 0) behavior. |
💡 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.
* `getVarLen` should be re-initialized before the second `WP11_Object_GetAttr` call. * Session leaks if `C_Login` fails.
dgarske
approved these changes
Mar 19, 2026
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.
CKM_RSA_PKCS_PSSmechanism info size