1257132 - Dereference after null check in tls13hkdf.c

Status: RESOLVED INVALID

(NSS :: Libraries, defect)

Comment 1

[Franziskus Kiefer [:franziskus]]

Attachment: tls13hkdf-coverity.patch

printing handshakeHash only if not null

Comment 2

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 8731175 [details] [diff] [review]
tls13hkdf-coverity.patch

Review of attachment 8731175 [details] [diff] [review]:
-----------------------------------------------------------------

::: lib/ssl/tls13hkdf.c
@@ +176,5 @@
>      }
>      PRINT_KEY(60, (NULL, "PRK", prk));
> +    if (handshakeHash) {
> +        PRINT_BUF(60, (NULL, "Hash", handshakeHash, handshakeHashLen));
> +    }

Was looking at that too and wondered if it would be helpful to print that Hash is NULL?

Comment 3

[Eric Rescorla (:ekr)]

I don't believe this is actually a null pointer dereference because PRINT_BUF
handles it correctly:

https://dxr.mozilla.org/mozilla-central/source/security/nss/lib/ssl/ssltrace.c#37

It also generates the right output:
6166: SSL: Hash [Len: 0]

Comment 4

[Franziskus Kiefer [:franziskus]]

(In reply to Eric Rescorla (:ekr) from comment #3)
> I don't believe this is actually a null pointer dereference because PRINT_BUF
> handles it correctly:
> 
> https://dxr.mozilla.org/mozilla-central/source/security/nss/lib/ssl/ssltrace.
> c#37
> 
> It also generates the right output:
> 6166: SSL: Hash [Len: 0]

right, as long as the len is always correct that works. Since this is only used in TRACE I think that's fine.