Commit 90378d3 crypto: Silence a Coverity warning in OpenSSL version of sss_hmac_sha1()

1 file Authored by jhrozek 2 months ago , Committed by fidencio 2 months ago ,
crypto: Silence a Coverity warning in OpenSSL version of sss_hmac_sha1()

It looks like the case where the key_len was exactly 64 was Confusing
Coverity. The trace looks like this:

  2. Condition key_len > 64, taking false branch.
  3. cond_at_most: Checking key_len > 64UL implies that key_len may be up to 64 on the false branch.
49    if (key_len > HMAC_SHA1_BLOCKSIZE) {
50        /* keys longer than blocksize are shortened */
51        if (!EVP_DigestInit_ex(ctx, EVP_sha1(), NULL)) {
52            ret = EIO;
53            goto done;
54        }
55
56        EVP_DigestUpdate(ctx, (const unsigned char *)key, key_len);
57        EVP_DigestFinal_ex(ctx, ikey, &res_len);
58        memset(ikey + SSS_SHA1_LENGTH, 0, HMAC_SHA1_BLOCKSIZE - SSS_SHA1_LENGTH);
59    } else {
60        /* keys shorter than blocksize are zero-padded */
61        memcpy(ikey, key, key_len);
  CID 18054 (#1 of 1): Out-of-bounds read (OVERRUN)4. overrun-local: Overrunning array of 64 bytes at byte offset 64 by dereferencing pointer ikey + key_len. [Note: The source code implementation of the function has been overridden by a builtin model.]
62        memset(ikey + key_len, 0, HMAC_SHA1_BLOCKSIZE - key_len);
63    }

I think this is a false positive because then HMAC_SHA1_BLOCKSIZE-key_len
will be 0, so ikey+key_len will not be dereferenced at all, but let's be
helpful to Coverity and make sure the branch is not evaluated at all if
key_len == HMAC_SHA1_BLOCKSIZE.

Reviewed-by: Sumit Bose <sbose@redhat.com>

    
 1 @@ -59,7 +59,9 @@
 2       } else {
 3           /* keys shorter than blocksize are zero-padded */
 4           memcpy(ikey, key, key_len);
 5 -         memset(ikey + key_len, 0, HMAC_SHA1_BLOCKSIZE - key_len);
 6 +         if (key_len < HMAC_SHA1_BLOCKSIZE) {
 7 +             memset(ikey + key_len, 0, HMAC_SHA1_BLOCKSIZE - key_len);
 8 +         }
 9       }
10   
11       /* HMAC(key, msg) = HASH(key XOR opad, HASH(key XOR ipad, msg)) */