Don't read uninitialised data for short session IDs.
authorDavid Benjamin <davidben@google.com>
Thu, 9 Feb 2017 20:13:13 +0000 (15:13 -0500)
committerRichard Levitte <levitte@openssl.org>
Mon, 13 Feb 2017 10:47:13 +0000 (11:47 +0100)
commitf26015db3337470cc8396b9283194df96aff7d1b
tree6112b5670a3d74dd121f86fd57987f7dcba8ee32
parent28903862fb0d6351dd88522950375ed44c10d1ba
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.

(cherry picked from commit bd5d27c1c6d3f83464ddf5124f18a2cac2cbb37f)
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2583)
ssl/ssl_lib.c