Initialize C_Initialize's reader list to NULs.
valgrind --track-origins=true says:
==13529== Conditional jump or move depends on uninitialised value(s)
==13529== at 0x4AF92D1: PK11_MakeString (pk11slot.c:1073)
==13529== by 0x4AFA5AA: PK11_InitSlot (pk11slot.c:1456)
==13529== by 0x4AE315E: secmod_LoadPKCS11Module (pk11load.c:563)
==13529== by 0x4AEF68C: SECMOD_LoadModule (pk11pars.c:1838)
==13529== by 0x4AEF7C7: SECMOD_LoadModule (pk11pars.c:1874)
==13529== by 0x4ABCB6A: nss_InitModules (nssinit.c:464)
==13529== by 0x4ABCB6A: nss_Init (nssinit.c:689)
==13529== by 0x4ABD17C: NSS_Init (nssinit.c:824)
==13529== by 0x4059C0: main (pesign.c:354)
==13529== Uninitialised value was created by a stack allocation
==13529== at 0x484D175: C_Initialize (in /usr/lib64/libykcs11.so.1.5.0)
This is the result of a combination of two problems. In
ykcs11/utils.c:parse_readers(), the code does:
for (i = 0; i < len; i++)
if (readers[i] == '\0' && i != len - 1) {
But in ykcs11/ykcs11.c:C_Initialize(), the parts of readers[] that are
initialized are only the parts that have been populated; the rest of
the array is still just whatever value is on the stack. Additionally,
in lib/ykpiv.c:ykpiv_list_readers(), which populates the array, the
length is updated only in the case where the buffer is smaller than the
data, not when there is additional buffer but no data:
if (num_readers > *len) {
num_readers = (pcsc_word)*len;
}
The result is that if the amount of reader data is smaller than 2048
bytes, PK11_InitSlot() will try to find reader data in the rest of the
array, which has not been initialized.
This patch adds an initialization for the data to set it all '\0', and
also updates the length when there is excess buffer available.
Signed-off-by: Peter Jones <pjones@redhat.com>
This commit is contained in:
@@ -520,6 +520,8 @@ ykpiv_rc ykpiv_list_readers(ykpiv_state *state, char *readers, size_t *len) {
|
||||
|
||||
if (num_readers > *len) {
|
||||
num_readers = (pcsc_word)*len;
|
||||
} else if (num_readers < *len) {
|
||||
*len = (size_t)num_readers;
|
||||
}
|
||||
|
||||
rc = SCardListReaders(state->context, NULL, readers, &num_readers);
|
||||
|
||||
Reference in New Issue
Block a user