Increase wbuf by one byte to fix the bug reported by

Eric Day <eday@concentric.net> to openssl-dev@openssl.org,
Message-ID: <20001218013437.A5526@concentric.net>

diff --git a/CHANGES b/CHANGES
index 888b6f435662316da3c114ca455e2af72a1c74de..f47d6ed81c726e7254393b739d8a78793d9e168e 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -4,6 +4,11 @@
 
  Changes between 0.9.6 and 0.9.6a  [xx XXX 2000]
 
+  *) Increase s2->wbuf allocation by one byte in ssl2_new (ssl/s2_lib.c).
+     Otherwise do_ssl_write (ssl/s2_pkt.c) will write beyond buffer limits
+     when writing a 32767 byte record.
+     [Bodo Moeller; problem reported by Eric Day <eday@concentric.net>]
+
   *) rand_win.c fix for Borland C.
      [Ulf Möller]
  

diff --git a/ssl/s2_lib.c b/ssl/s2_lib.c
index 129ed89d970448a84cefd0b3fcff9423f0186917..a89958607cfd2800e03f5d0e90535891fa0ac704 100644 (file)
--- a/ssl/s2_lib.c
+++ b/ssl/s2_lib.c
@@ -270,10 +270,16 @@ int ssl2_new(SSL *s)
        if ((s2=OPENSSL_malloc(sizeof *s2)) == NULL) goto err;
        memset(s2,0,sizeof *s2);
 
+#if SSL2_MAX_RECORD_LENGTH_3_BYTE_HEADER + 3 > SSL2_MAX_RECORD_LENGTH_2_BYTE_HEADER + 2
+#  error "assertion failed"
+#endif
+
        if ((s2->rbuf=OPENSSL_malloc(
                SSL2_MAX_RECORD_LENGTH_2_BYTE_HEADER+2)) == NULL) goto err;
+       /* wbuf needs one byte more because when using two-byte headers,
+        * we leave the first byte unused in do_ssl_write (s2_pkt.c) */
        if ((s2->wbuf=OPENSSL_malloc(
-               SSL2_MAX_RECORD_LENGTH_2_BYTE_HEADER+2)) == NULL) goto err;
+               SSL2_MAX_RECORD_LENGTH_2_BYTE_HEADER+3)) == NULL) goto err;
        s->s2=s2;
 
        ssl2_clear(s);

diff --git a/ssl/s2_pkt.c b/ssl/s2_pkt.c
index 6081dd7b4709bf475deb1c486b054d7c2df4622d..e2499083e9531e4a82ab92668d7b61117a023bc2 100644 (file)
--- a/ssl/s2_pkt.c
+++ b/ssl/s2_pkt.c
@@ -541,6 +541,9 @@ static int do_ssl_write(SSL *s, const unsigned char *buf, unsigned int len)
                {
                bs=EVP_CIPHER_CTX_block_size(s->enc_read_ctx);
                j=len+mac_size;
+               /* Two-byte headers allow for a larger record length than
+                * three-byte headers, but we can't use them if we need
+                * padding or if we have to set the escape bit. */
                if ((j > SSL2_MAX_RECORD_LENGTH_3_BYTE_HEADER) &&
                        (!s->s2->escape))
                        {
@@ -560,7 +563,7 @@ static int do_ssl_write(SSL *s, const unsigned char *buf, unsigned int len)
                        s->s2->three_byte_header=0;
                        p=0;
                        }
-               else /* 3 byte header */
+               else /* we may have to use a 3 byte header */
                        {
                        /*len=len; */
                        p=(j%bs);
@@ -574,7 +577,7 @@ static int do_ssl_write(SSL *s, const unsigned char *buf, unsigned int len)
        /* mac_size is the number of MAC bytes
         * len is the number of data bytes we are going to send
         * p is the number of padding bytes
-        * if p == 0, it is a 2 byte header */
+        * (if it is a two-byte header, then p == 0) */
 
        s->s2->wlength=len;
        s->s2->padding=p;

diff --git a/ssl/ssl2.h b/ssl/ssl2.h
index df7d03c18f70d02dbd425ea4e292acd34af0e0bf..f8b56afb6b20ef37a005f975aecbbbbbd3de2997 100644 (file)
--- a/ssl/ssl2.h
+++ b/ssl/ssl2.h
@@ -134,11 +134,11 @@ extern "C" {
 /* Upper/Lower Bounds */
 #define SSL2_MAX_MASTER_KEY_LENGTH_IN_BITS     256
 #ifdef MPE
-#define SSL2_MAX_RECORD_LENGTH_2_BYTE_HEADER   (unsigned int)29998
+#define SSL2_MAX_RECORD_LENGTH_2_BYTE_HEADER   29998u
 #else
-#define SSL2_MAX_RECORD_LENGTH_2_BYTE_HEADER   (unsigned int)32767 
+#define SSL2_MAX_RECORD_LENGTH_2_BYTE_HEADER   32767u  /* 2^15-1 */
 #endif
-#define SSL2_MAX_RECORD_LENGTH_3_BYTE_HEADER   16383 /**/
+#define SSL2_MAX_RECORD_LENGTH_3_BYTE_HEADER   16383 /* 2^14-1 */
 
 #define SSL2_CHALLENGE_LENGTH  16
 /*#define SSL2_CHALLENGE_LENGTH        32 */