Issue discovered by asan about libdsm HOT 5 CLOSED

@tguillem, @elthariel - opinions? :)

from libdsm.

Well, it authentication fails with the change, then the change is wrong, no?

from libdsm.

It didn't work before the change and the code was as I understood it wrong.
I'm trying to find the reason why browsing of smbx servers with VLC for iOS and tvOS only works on a full moon…
Also clang static analyzer does find different issues like use-after-free among other things.

from libdsm.

Considering the fact that first parameter of HMAC_MD5 is a pointer (const void *key), there are some function calls that are incorrect :
in smb_ntlm2_hash :

HMAC_MD5(hash_v1, SMB_NTLM_HASH_SIZE, data, data_len, hash);

should be

HMAC_MD5(&hash_v1, SMB_NTLM_HASH_SIZE, data, data_len, hash);

in smb_ntlm2_session_key(smb_ntlmh *hash_v2, ...

HMAC_MD5(&hash_v2, 16, ntlm2, 16, &hmac_ntlm2);

should be

HMAC_MD5(hash_v2, 16, ntlm2, 16, &hmac_ntlm2);

One other point, the session state "SMB_STATE_SESSION_OK" at the end of authentication is only possible when going through smb_session_login_ntlm, if we login using smb_session_login_spnego, the final session state is SMB_STATE_DIALECT_OK.

One last point regarding "HMAC_MD5" function itself :
This part of the code looks suspicious :

    // This is Microsoft variation of HMAC_MD5 for NTLMv2
    // It seems they truncate over-sized keys instead of rehashing
    if (key_len > 64)
        key_len = 64;
    else
    {
        memcpy(key_pad, key, key_len);
        memset(key_pad + key_len, 0, 64 - key_len);
    }

The point is that key_len is only used in the else statement ! And it means that if the key size is above 64 bytes, the key will not be copied in "key_pad" buffer, and the following code will work with an uninitialised buffer ...
I guess that this code should be

    // This is Microsoft variation of HMAC_MD5 for NTLMv2
    // It seems they truncate over-sized keys instead of rehashing
    if (key_len > 64)
        key_len = 64;

    memcpy(key_pad, key, key_len);
    memset(key_pad + key_len, 0, 64 - key_len);

from libdsm.

Fixed in 8ec9870.

from libdsm.