root
/
openssl
mirror of https://github.com/openssl/openssl.git
1
0
Fork 0
Code Issues Actions 7 Packages Projects Releases Wiki Activity

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 bd5d27c1c6)
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2583)
This commit is contained in:
David Benjamin 2017-02-09 15:13:13 -05:00 committed by Richard Levitte
parent 28903862fb
commit f26015db33
1 changed files with 12 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
ssl/ssl_lib.c
View File

@ -1838,13 +1838,21 @@ int SSL_export_keying_material(SSL *s, unsigned char *out, size_t olen,
static unsigned long ssl_session_hash(const SSL_SESSION *a)
{
const unsigned char *session_id = a->session_id;
unsigned long l;
unsigned char tmp_storage[4];
if (a->session_id_length < sizeof(tmp_storage)) {
memset(tmp_storage, 0, sizeof(tmp_storage));
memcpy(tmp_storage, a->session_id, a->session_id_length);
session_id = tmp_storage;
}
l = (unsigned long)
((unsigned int)a->session_id[0]) |
((unsigned int)a->session_id[1] << 8L) |
((unsigned long)a->session_id[2] << 16L) |
((unsigned long)a->session_id[3] << 24L);
((unsigned long)session_id[0]) |
((unsigned long)session_id[1] << 8L) |
((unsigned long)session_id[2] << 16L) |
((unsigned long)session_id[3] << 24L);
return (l);
}