Remove a reachable assert from ssl3_write_bytes
authorMatt Caswell <matt@openssl.org>
Fri, 30 Oct 2015 17:01:01 +0000 (17:01 +0000)
committerMatt Caswell <matt@openssl.org>
Mon, 2 Nov 2015 14:29:37 +0000 (14:29 +0000)
A buggy application that call SSL_write with a different length after a
NBIO event could cause an OPENSSL_assert to be reached. The assert is not
actually necessary because there was an explicit check a little further
down that would catch this scenario. Therefore remove the assert an move
the check a little higher up.

Reviewed-by: Rich Salz <rsalz@openssl.org>
ssl/record/rec_layer_s3.c

index e59c203..c9f1b71 100644 (file)
@@ -455,20 +455,7 @@ int ssl3_write_bytes(SSL *s, int type, const void *buf_, int len)
     }
 
     s->rwstate = SSL_NOTHING;
-    OPENSSL_assert(s->rlayer.wnum <= INT_MAX);
     tot = s->rlayer.wnum;
-    s->rlayer.wnum = 0;
-
-    if (SSL_in_init(s) && !ossl_statem_get_in_handshake(s)) {
-        i = s->handshake_func(s);
-        if (i < 0)
-            return (i);
-        if (i == 0) {
-            SSLerr(SSL_F_SSL3_WRITE_BYTES, SSL_R_SSL_HANDSHAKE_FAILURE);
-            return -1;
-        }
-    }
-
     /*
      * ensure that if we end up with a smaller value of data to write out
      * than the the original len from a write which didn't complete for
@@ -478,9 +465,22 @@ int ssl3_write_bytes(SSL *s, int type, const void *buf_, int len)
      * promptly send beyond the end of the users buffer ... so we trap and
      * report the error in a way the user will notice
      */
-    if (len < tot) {
+    if ((unsigned int)len < s->rlayer.wnum) {
         SSLerr(SSL_F_SSL3_WRITE_BYTES, SSL_R_BAD_LENGTH);
-        return (-1);
+        return -1;
+    }
+
+
+    s->rlayer.wnum = 0;
+
+    if (SSL_in_init(s) && !ossl_statem_get_in_handshake(s)) {
+        i = s->handshake_func(s);
+        if (i < 0)
+            return (i);
+        if (i == 0) {
+            SSLerr(SSL_F_SSL3_WRITE_BYTES, SSL_R_SSL_HANDSHAKE_FAILURE);
+            return -1;
+        }
     }
 
     /*