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.

BUG=586800

Change-Id: I5f59f245b51477d6d4fa2cdc20d40bb6b4a3eae7
Reviewed-on: https://boringssl-review.googlesource.com/7150
Reviewed-by: David Benjamin <davidben@google.com>
This commit is contained in:
Adam Langley 2016-02-15 14:01:10 -08:00 committed by David Benjamin
parent f48fcaf901
commit e976e4349d

View File

@ -181,12 +181,21 @@ int SSL_library_init(void) {
return 1;
}
static uint32_t ssl_session_hash(const SSL_SESSION *a) {
static uint32_t ssl_session_hash(const SSL_SESSION *sess) {
const uint8_t *session_id = sess->session_id;
uint8_t tmp_storage[sizeof(uint32_t)];
if (sess->session_id_length < sizeof(tmp_storage)) {
memset(tmp_storage, 0, sizeof(tmp_storage));
memcpy(tmp_storage, sess->session_id, sess->session_id_length);
session_id = tmp_storage;
}
uint32_t hash =
((uint32_t)a->session_id[0]) |
((uint32_t)a->session_id[1] << 8) |
((uint32_t)a->session_id[2] << 16) |
((uint32_t)a->session_id[3] << 24);
((uint32_t)session_id[0]) |
((uint32_t)session_id[1] << 8) |
((uint32_t)session_id[2] << 16) |
((uint32_t)session_id[3] << 24);
return hash;
}