From ffb772b401a7e4d8ff166372a160eb0da562824c Mon Sep 17 00:00:00 2001 From: miod Date: Thu, 7 Aug 2014 19:46:31 +0000 Subject: When you expect a function to return a particular value, don't put a comment saying that you expect it to return that value and compare it against zero because it is supposedly faster, for this leads to bugs (especially given the high rate of sloppy cut'n'paste within ssl3 and dtls1 routines in this library). Instead, compare for the exact value it ought to return upon success. ok deraadt@ --- lib/libssl/s3_both.c | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) (limited to 'lib/libssl/s3_both.c') diff --git a/lib/libssl/s3_both.c b/lib/libssl/s3_both.c index 500387e3720..afcaca3c434 100644 --- a/lib/libssl/s3_both.c +++ b/lib/libssl/s3_both.c @@ -1,4 +1,4 @@ -/* $OpenBSD: s3_both.c,v 1.26 2014/07/10 08:51:14 tedu Exp $ */ +/* $OpenBSD: s3_both.c,v 1.27 2014/08/07 19:46:31 miod Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -161,7 +161,7 @@ ssl3_send_finished(SSL *s, int a, int b, const char *sender, int slen) p = &(d[4]); i = s->method->ssl3_enc->final_finish_mac(s, - sender, slen, s->s3->tmp.finish_md); + sender, slen, s->s3->tmp.finish_md); if (i == 0) return 0; s->s3->tmp.finish_md_len = i; @@ -171,15 +171,14 @@ ssl3_send_finished(SSL *s, int a, int b, const char *sender, int slen) /* Copy the finished so we can use it for renegotiation checks */ + OPENSSL_assert(i <= EVP_MAX_MD_SIZE); if (s->type == SSL_ST_CONNECT) { - OPENSSL_assert(i <= EVP_MAX_MD_SIZE); memcpy(s->s3->previous_client_finished, - s->s3->tmp.finish_md, i); + s->s3->tmp.finish_md, i); s->s3->previous_client_finished_len = i; } else { - OPENSSL_assert(i <= EVP_MAX_MD_SIZE); memcpy(s->s3->previous_server_finished, - s->s3->tmp.finish_md, i); + s->s3->tmp.finish_md, i); s->s3->previous_server_finished_len = i; } @@ -216,7 +215,7 @@ ssl3_take_mac(SSL *s) } s->s3->tmp.peer_finish_md_len = s->method->ssl3_enc->final_finish_mac(s, - sender, slen, s->s3->tmp.peer_finish_md); + sender, slen, s->s3->tmp.peer_finish_md); } #endif @@ -250,7 +249,7 @@ ssl3_get_finished(SSL *s, int a, int b) p = (unsigned char *)s->init_msg; i = s->s3->tmp.peer_finish_md_len; - if (i != n) { + if (i != n || i > EVP_MAX_MD_SIZE) { al = SSL_AD_DECODE_ERROR; SSLerr(SSL_F_SSL3_GET_FINISHED, SSL_R_BAD_DIGEST_LENGTH); goto f_err; @@ -265,14 +264,12 @@ ssl3_get_finished(SSL *s, int a, int b) /* Copy the finished so we can use it for renegotiation checks */ if (s->type == SSL_ST_ACCEPT) { - OPENSSL_assert(i <= EVP_MAX_MD_SIZE); memcpy(s->s3->previous_client_finished, - s->s3->tmp.peer_finish_md, i); + s->s3->tmp.peer_finish_md, i); s->s3->previous_client_finished_len = i; } else { - OPENSSL_assert(i <= EVP_MAX_MD_SIZE); memcpy(s->s3->previous_server_finished, - s->s3->tmp.peer_finish_md, i); + s->s3->tmp.peer_finish_md, i); s->s3->previous_server_finished_len = i; } -- cgit v1.2.3-59-g8ed1b