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

1 file Authored by jhrozek 4 months ago , Committed by fidencio 4 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>