Don't read uninitialised data for short session IDs.
While it's always safe to read |SSL_MAX_SSL_SESSION_ID_LENGTH| bytes from an |SSL_SESSION|'s |session_id| array, the hash function would do so with without considering if all those bytes had been written to. This change checks |session_id_length| before possibly reading uninitialised memory. Since the result of the hash function was already attacker controlled, and since a lookup of a short session ID will always fail, it doesn't appear that this is anything more than a clean up. In particular, |ssl_get_prev_session| uses a stack-allocated placeholder |SSL_SESSION| as a lookup key, so the |session_id| array may be uninitialised. This was originally found with libFuzzer and MSan in https://boringssl.googlesource.com/boringssl/+/e976e4349d693b4bbb97e1694f45be5a1b22c8c7 , then by Robert Swiecki with honggfuzz and MSan here. Thanks to both. Reviewed-by: Geoff Thorpe <geoff@openssl.org> Reviewed-by: Rich Salz <rsalz@openssl.org> (Merged from https://github.com/openssl/openssl/pull/2583)
parent
76e624a0
Please register or sign in to comment