276181 - PK11_HashBuf returns SECSuccess given a NULL digest

Status: RESOLVED DUPLICATE of bug 276311

(NSS :: Libraries, defect)

Description

[Jason Reid]

/* null digest */
        digest = (unsigned char *)malloc((MD5_LENGTH + 1)*sizeof(unsigned char));
        secStatus = PK11_HashBuf(SEC_OID_MD5, NULL, buffer, strlen(buffer));
        if (SECSuccess ==  secStatus ) {
                fprintf(stderr,
"ERROR: PK11_HashBuf succeeded for a NULL digest\n");
        } else {
                fprintf(stderr,
"PK11_HashBuf failed for a NULL digest. (expected)\n");
        }
        free(digest);

PK11_HashBuf returns SECSuccess if given a NULL value for the digest buffer.
Since it can not write the digest out successfully, it should return 
SECFailure.

> gdb ./hash-tests
GNU gdb 5.3.92
Copyright 2003 Free Software Foundation, Inc.
GDB is free software, covered by the GNU General Public License, and you are
welcome to change it and/or distribute copies of it under certain conditions.
Type "show copying" to see the conditions.
There is absolutely no warranty for GDB.  Type "show warranty" for details.
This GDB was configured as "i586-suse-linux"...
(gdb) break 143
Breakpoint 1 at 0x8048b9d: file hash-tests.c, line 143.
(gdb) run
Starting program: /home/jmr/work/nss/tests/Hashing/hash-tests
[New Thread 16384 (LWP 3393)]
0xdeadbeef hash tests
MD2= 17682ba795d60a2e740ba5f4539007a4
MD5= 4f41243847da693a4f356c0486114bc6
SHA1= f49cf6381e322b147053b74e4500af8533ac1e4c
SHA256= 2baf1f40105d9501fe319a8ec463fdf4325a2a5df445adf3f572f626253678c9
SHA384=
906d9fe92a5e0ad7e020842127f7970b7d2adff3a91106927b592cf157633d86d0be6ab78f5d398e53f705643ccbfb78
SHA512=
113a3bc783d851fc0373214b19ea7be9fa3de541ecb9fe026d52c603e8ea19c174cc0e9705f8b90d312212c0c3a6d8453ddfb3e3141409cf4bedc8ef033590b4
[Switching to Thread 16384 (LWP 3393)]

Breakpoint 1, main (argc=1, argv=0xbffff1e4) at hash-tests.c:143
143             secStatus = PK11_HashBuf(SEC_OID_MD5, NULL, buffer, strlen(buffer));
(gdb) print secStatus
$1 = SECSuccess
(gdb) next
144             if (SECSuccess ==  secStatus ) {
(gdb) print secStatus
$2 = SECSuccess
(gdb) quit
The program is running.  Exit anyway? (y or n) y

Comment 1

[Wan-Teh Chang]

I believe this is because of the following code in
the softoken:

/* NSC_DigestFinal finishes a multiple-part message-digesting operation. */
CK_RV NSC_DigestFinal(CK_SESSION_HANDLE hSession,CK_BYTE_PTR pDigest,
                                                CK_ULONG_PTR pulDigestLen)
{
    PK11Session *session;
    PK11SessionContext *context;
    unsigned int maxout = *pulDigestLen;
    unsigned int digestLen;
    CK_RV crv;

    /* make sure we're legal */
    crv = pk11_GetContext(hSession, &context, PK11_HASH, PR_TRUE, &session);
    if (crv != CKR_OK) return crv;

    if (pDigest != NULL) {
        (*context->end)(context->cipherInfo, pDigest, &digestLen, maxout);
        *pulDigestLen = digestLen;
        pk11_SetContextByType(session, PK11_HASH, NULL);
        pk11_FreeContext(context);
    } else {
        *pulDigestLen = context->maxLen;
    }

    pk11_FreeSession(session);
    return CKR_OK;
}

Note that we simply do
    *pulDigestLen = context->maxLen;
and return CKR_OK if pDigest is NULL.
(NSC_Digest has similar code.)

Bob, why is that?  Is a null pDigest
a valid input and what does it mean?

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

The PKCS11 C_DigestFInal API explicitly uses a NULL output buffer pointer 
as a means by which the caller can ask the PKCS11 module what size buffer
is needed for the output.  A caller that uses this method would typically
call C_DigestFinal twice, once with a NULL buffer pointer to get the size
of the buffer to allocate, and a second time with a pointer to a freshly
allocated buffer of that size.  

But the PK11_HashBuf function has no way to return the desired output
length.  IMO, it should return an error when called with a NULL output
buffer pointer.  But I'd give this no more priority than P3 or P4.

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

This is another case of a public function in pk11cxt.c not checking its
arguments for NULL pointers.  I'm duping this bug to bug 276311.

*** This bug has been marked as a duplicate of 276311 ***