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@

1 files changed, 9 insertions, 12 deletions

diff --git a/src/lib/libssl/s3_both.c b/src/lib/libssl/s3_both.c
index 500387e372..afcaca3c43 100644
--- a/src/lib/libssl/s3_both.c
+++ b/src/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;
         }