Closed Bug 920899 Opened 8 years ago Closed 8 years ago

fipstest has bad memset in hmac_test


(NSS :: Test, defect, P2)



(Not tracked)



(Reporter: vapier, Assigned: vapier)



(1 file, 1 obsolete file)

Attached patch fipstest.patch (obsolete) — Splinter Review
when compiling with recent versions of gcc, this warning is issued:

fipstest.c: In function ‘hmac_test’:
fipstest.c:3680:35: warning: argument to ‘sizeof’ in ‘memset’ call is the same expression as the destination; did you mean to provide an explicit length? [-Wsizeof-pointer-memaccess]
             memset(msg, 0, sizeof msg);  

normally in this code base, buffers are declared on the stack like so:
    unsigned char HMAC[HASH_LENGTH_MAX];
that way when the code does the memset like it does for msg, it works.

but this particular case allocs msg dynamically:
    unsigned int msgLen = 128;    /* the length of the input  */
                                  /*  Message is always 128 Bytes */
    unsigned char *msg = NULL;    /* holds the message to digest.*/
    msg = PORT_ZAlloc(msgLen);
    memset(msg, 0, msgLen);

so this later memset in the code which does sizeof msg is wrong -- it only zereos out the first sizeof(void*) bytes instead of all 128.

attached patch fixes this.  it also fixes an earlier memset in this func which is run before the NULL pointer check.  it's a possible NULL pointer deref.
Mike: thank you for the patch. The first memset is not necessary
because PORT_ZAlloc (a simple wrapper around calloc) zeros the
returned memory.

In fact, the |msg| buffer can be allocated on the stack like the
|key| and |HMAC| buffers.
Attachment #810375 - Attachment is obsolete: true
Attachment #811335 - Flags: review+
Attachment #811335 - Flags: checked-in+
Assignee: nobody → vapier
Closed: 8 years ago
Priority: -- → P2
Resolution: --- → FIXED
Target Milestone: --- → 3.15.3
Version: 3.15.1 → trunk
changing target milestone to 3.15.4
Target Milestone: 3.15.3 → 3.15.4
You need to log in before you can comment on or make changes to this bug.