From 99ceb2d40c70fa8405151669578afb3be1d7c8c6 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Mon, 27 Apr 2015 15:41:42 +0100 Subject: [PATCH] Add sanity check in ssl3_cbc_digest_record For SSLv3 the code assumes that |header_length| > |md_block_size|. Whilst this is true for all SSLv3 ciphersuites, this fact is far from obvious by looking at the code. If this were not the case then an integer overflow would occur, leading to a subsequent buffer overflow. Therefore I have added an explicit sanity check to ensure header_length is always valid. Thanks to Kevin Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3 Solutions) for reporting this issue. Reviewed-by: Andy Polyakov (cherry picked from commit 29b0a15a480626544dd0c803d5de671552544de6) --- ssl/s3_cbc.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/ssl/s3_cbc.c b/ssl/s3_cbc.c index f31dc046f3..c43402d4a1 100644 --- a/ssl/s3_cbc.c +++ b/ssl/s3_cbc.c @@ -639,12 +639,22 @@ void ssl3_cbc_digest_record(const EVP_MD_CTX *ctx, if (k > 0) { if (is_sslv3) { + unsigned overhang; + /* * The SSLv3 header is larger than a single block. overhang is * the number of bytes beyond a single block that the header - * consumes: either 7 bytes (SHA1) or 11 bytes (MD5). + * consumes: either 7 bytes (SHA1) or 11 bytes (MD5). There are no + * ciphersuites in SSLv3 that are not SHA1 or MD5 based and + * therefore we can be confident that the header_length will be + * greater than |md_block_size|. However we add a sanity check just + * in case */ - unsigned overhang = header_length - md_block_size; + if (header_length <= md_block_size) { + /* Should never happen */ + return; + } + overhang = header_length - md_block_size; md_transform(md_state.c, header); memcpy(first_block, header + md_block_size, overhang); memcpy(first_block + overhang, data, md_block_size - overhang); -- 2.25.1