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

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

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.

Comment 1

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Attachment: 1313496-PK11_FindSlotsByNames-double-refcount.diff

Are there existing tests for these sorts of things? (If so, where?)

Comment 2

[Tim Taubert [:ttaubert] (inactive)]

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.

Comment 3

[Tim Taubert [:ttaubert] (inactive)]

Thank you! Landed:

https://hg.mozilla.org/projects/nss/rev/153aed6a4f2b