Issue #1331: Off-by-one error in sss_hmac_sha1 - sssd

SSSD / sssd

#1331 Off-by-one error in sss_hmac_sha1

Closed: Fixed None Opened 12 years ago by jhrozek.

ikey is defined as unsigned char ikey[HMAC_SHA1_BLOCKSIZE]. If key_len is exactly HMAC_SHA1_BLOCKSIZE, we would access the item with index HMAC_SHA1_BLOCKSIZE. I actually tested this and memset survives fine, most probably because the number of items to set is 0. We can't rely on the behaviour and should fix this bug.

At conditional (3): "key_len > 64UL" taking the false branch.
57    if (key_len > HMAC_SHA1_BLOCKSIZE) {
58        /* keys longer than blocksize are shortened */
59        HASH_Begin(sha1);
60        HASH_Update(sha1, key, key_len);
61        HASH_End(sha1, ikey, &res_len, SSS_SHA1_LENGTH);
62        memset(ikey + SSS_SHA1_LENGTH, 0, HMAC_SHA1_BLOCKSIZE - SSS_SHA1_LENGTH);
63    } else {
64        /* keys shorter than blocksize are zero-padded */
65        memcpy(ikey, key, key_len);
CID 12623: Out-of-bounds read (OVERRUN_STATIC)Overrunning static array of size 64 bytes at byte position 64 by accessing with pointer "&ikey[key_len]" through dereference in call to "memset". (The dereference is assumed on the basis of the 'nonnull' parameter attribute.)
Note: These bugs are often difficult to see at first glance. Coverity recommends a close inspection of the events leading to this overrun.
66        memset(ikey + key_len, 0, HMAC_SHA1_BLOCKSIZE - key_len);
67    }