PK11_FindSlotsByNames double-references the internal key slot if given any null/empty inputs

RESOLVED FIXED in 3.28

Status

NSS
Libraries
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: keeler, Assigned: keeler)

Tracking

trunk
3.28

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

PK11_FindSlotsByNames double-references the internal key slot if given any null/empty inputs.

pk11slot.c:

    if (((NULL == dllName) || (0 == *dllName)) &&
        ((NULL == slotName) || (0 == *slotName)) &&
        ((NULL == tokenName) || (0 == *tokenName))) {
        /* default to softoken */
        PK11_AddSlotToList(slotList, PK11_GetInternalKeySlot(), PR_TRUE);
        return slotList;
    }

PK11_GetInternalKeySlot increments the refcount on the slot. PK11_AddSlotToList also increments the refcount on the slot. Luckily the fix is pretty straightforward. I'll attach a patch shortly.
Created attachment 8805268 [details] [diff] [review]
1313496-PK11_FindSlotsByNames-double-refcount.diff

Are there existing tests for these sorts of things? (If so, where?)
Attachment #8805268 - Flags: review?(ttaubert)
Comment on attachment 8805268 [details] [diff] [review]
1313496-PK11_FindSlotsByNames-double-refcount.diff

Review of attachment 8805268 [details] [diff] [review]:
-----------------------------------------------------------------

(In reply to David Keeler [:keeler] (use needinfo?) from comment #1)
> Are there existing tests for these sorts of things? (If so, where?)

No... I wish. They might be accidentally covered by other tests but there's not a lot of code (or any?) that tests the PK11_* API. This could be found by the LeakSanitizer - if we run into it. `PK11_FindSlotsByNames()` is only exported, there's are no call sites in NSS itself.
Attachment #8805268 - Flags: review?(ttaubert) → review+
Thank you! Landed:

https://hg.mozilla.org/projects/nss/rev/153aed6a4f2b
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → 3.28
You need to log in before you can comment on or make changes to this bug.