PR: 2258
authorDr. Stephen Henson <steve@openssl.org>
Thu, 27 May 2010 12:41:20 +0000 (12:41 +0000)
committerDr. Stephen Henson <steve@openssl.org>
Thu, 27 May 2010 12:41:20 +0000 (12:41 +0000)
Submitted By: Ger Hobbelt <ger@hobbelt.com>

Base64 BIO fixes:

Use OPENSSL_assert() instead of assert().
Use memmove() as buffers overlap.
Fix write retry logic.

crypto/evp/bio_b64.c

index fa5cbc7eb1ffc68fa212a102856e092c55b6512e..72a2a67277a32e9b470fe24a8eec7b68fb1d5850 100644 (file)
@@ -64,7 +64,7 @@
 
 static int b64_write(BIO *h, const char *buf, int num);
 static int b64_read(BIO *h, char *buf, int size);
-/*static int b64_puts(BIO *h, const char *str); */
+static int b64_puts(BIO *h, const char *str);
 /*static int b64_gets(BIO *h, char *str, int size); */
 static long b64_ctrl(BIO *h, int cmd, long arg1, void *arg2);
 static int b64_new(BIO *h);
@@ -96,7 +96,7 @@ static BIO_METHOD methods_b64=
        BIO_TYPE_BASE64,"base64 encoding",
        b64_write,
        b64_read,
-       NULL, /* b64_puts, */
+       b64_puts,
        NULL, /* b64_gets, */
        b64_ctrl,
        b64_new,
@@ -127,6 +127,7 @@ static int b64_new(BIO *bi)
        bi->init=1;
        bi->ptr=(char *)ctx;
        bi->flags=0;
+       bi->num = 0;
        return(1);
        }
 
@@ -151,6 +152,8 @@ static int b64_read(BIO *b, char *out, int outl)
 
        if ((ctx == NULL) || (b->next_bio == NULL)) return(0);
 
+       BIO_clear_retry_flags(b);
+
        if (ctx->encode != B64_DECODE)
                {
                ctx->encode=B64_DECODE;
@@ -163,6 +166,7 @@ static int b64_read(BIO *b, char *out, int outl)
        /* First check if there are bytes decoded/encoded */
        if (ctx->buf_len > 0)
                {
+               OPENSSL_assert(ctx->buf_len >= ctx->buf_off);
                i=ctx->buf_len-ctx->buf_off;
                if (i > outl) i=outl;
                OPENSSL_assert(ctx->buf_off+i < (int)sizeof(ctx->buf));
@@ -184,7 +188,6 @@ static int b64_read(BIO *b, char *out, int outl)
        ret_code=0;
        while (outl > 0)
                {
-
                if (ctx->cont <= 0)
                        break;
 
@@ -195,7 +198,7 @@ static int b64_read(BIO *b, char *out, int outl)
                        {
                        ret_code=i;
 
-                       /* Should be continue next time we are called? */
+                       /* Should we continue next time we are called? */
                        if (!BIO_should_retry(b->next_bio))
                                {
                                ctx->cont=i;
@@ -285,19 +288,27 @@ static int b64_read(BIO *b, char *out, int outl)
                                continue;
                                }
                        else
+                       {
                                ctx->tmp_len=0;
                        }
-               /* If buffer isn't full and we can retry then
-                * restart to read in more data.
-                */
+               }
                else if ((i < B64_BLOCK_SIZE) && (ctx->cont > 0))
+               {
+                       /* If buffer isn't full and we can retry then
+                        * restart to read in more data.
+                        */
                        continue;
+               }
 
                if (BIO_get_flags(b) & BIO_FLAGS_BASE64_NO_NL)
                        {
                        int z,jj;
 
+#if 0
                        jj=(i>>2)<<2;
+#else
+                       jj = i & ~3; /* process per 4 */
+#endif
                        z=EVP_DecodeBlock((unsigned char *)ctx->buf,
                                (unsigned char *)ctx->tmp,jj);
                        if (jj > 2)
@@ -313,18 +324,15 @@ static int b64_read(BIO *b, char *out, int outl)
                         * number consumed */
                        if (jj != i)
                                {
-                               memcpy((unsigned char *)ctx->tmp,
-                                       (unsigned char *)&(ctx->tmp[jj]),i-jj);
+                               memmove(ctx->tmp, &ctx->tmp[jj], i-jj);
                                ctx->tmp_len=i-jj;
                                }
                        ctx->buf_len=0;
                        if (z > 0)
                                {
                                ctx->buf_len=z;
-                               i=1;
                                }
-                       else
-                               i=z;
+                       i=z;
                        }
                else
                        {
@@ -357,14 +365,16 @@ static int b64_read(BIO *b, char *out, int outl)
                outl-=i;
                out+=i;
                }
-       BIO_clear_retry_flags(b);
+       /* BIO_clear_retry_flags(b); */
        BIO_copy_next_retry(b);
        return((ret == 0)?ret_code:ret);
        }
 
 static int b64_write(BIO *b, const char *in, int inl)
        {
-       int ret=inl,n,i;
+       int ret=0;
+       int n;
+       int i;
        BIO_B64_CTX *ctx;
 
        ctx=(BIO_B64_CTX *)b->ptr;
@@ -379,6 +389,9 @@ static int b64_write(BIO *b, const char *in, int inl)
                EVP_EncodeInit(&(ctx->base64));
                }
 
+       OPENSSL_assert(ctx->buf_off < (int)sizeof(ctx->buf));
+       OPENSSL_assert(ctx->buf_len <= (int)sizeof(ctx->buf));
+       OPENSSL_assert(ctx->buf_len >= ctx->buf_off);
        n=ctx->buf_len-ctx->buf_off;
        while (n > 0)
                {
@@ -388,7 +401,10 @@ static int b64_write(BIO *b, const char *in, int inl)
                        BIO_copy_next_retry(b);
                        return(i);
                        }
+               OPENSSL_assert(i <= n);
                ctx->buf_off+=i;
+               OPENSSL_assert(ctx->buf_off <= (int)sizeof(ctx->buf));
+               OPENSSL_assert(ctx->buf_len >= ctx->buf_off);
                n-=i;
                }
        /* at this point all pending data has been written */
@@ -405,18 +421,19 @@ static int b64_write(BIO *b, const char *in, int inl)
                        {
                        if (ctx->tmp_len > 0)
                                {
+                               OPENSSL_assert(ctx->tmp_len <= 3);
                                n=3-ctx->tmp_len;
-                               /* There's a teoretical possibility for this */
+                               /* There's a theoretical possibility for this */
                                if (n > inl) 
                                        n=inl;
                                memcpy(&(ctx->tmp[ctx->tmp_len]),in,n);
                                ctx->tmp_len+=n;
+                               ret += n;
                                if (ctx->tmp_len < 3)
                                        break;
-                               ctx->buf_len=EVP_EncodeBlock(
-                                       (unsigned char *)ctx->buf,
-                                       (unsigned char *)ctx->tmp,
-                                       ctx->tmp_len);
+                               ctx->buf_len=EVP_EncodeBlock((unsigned char *)ctx->buf,(unsigned char *)ctx->tmp,ctx->tmp_len);
+                               OPENSSL_assert(ctx->buf_len <= (int)sizeof(ctx->buf));
+                               OPENSSL_assert(ctx->buf_len >= ctx->buf_off);
                                /* Since we're now done using the temporary
                                   buffer, the length should be 0'd */
                                ctx->tmp_len=0;
@@ -425,14 +442,16 @@ static int b64_write(BIO *b, const char *in, int inl)
                                {
                                if (n < 3)
                                        {
-                                       memcpy(&(ctx->tmp[0]),in,n);
+                                       memcpy(ctx->tmp,in,n);
                                        ctx->tmp_len=n;
+                                       ret += n;
                                        break;
                                        }
                                n-=n%3;
-                               ctx->buf_len=EVP_EncodeBlock(
-                                       (unsigned char *)ctx->buf,
-                                       (unsigned char *)in,n);
+                               ctx->buf_len=EVP_EncodeBlock((unsigned char *)ctx->buf,(const unsigned char *)in,n);
+                               OPENSSL_assert(ctx->buf_len <= (int)sizeof(ctx->buf));
+                               OPENSSL_assert(ctx->buf_len >= ctx->buf_off);
+                               ret += n;
                                }
                        }
                else
@@ -440,6 +459,9 @@ static int b64_write(BIO *b, const char *in, int inl)
                        EVP_EncodeUpdate(&(ctx->base64),
                                (unsigned char *)ctx->buf,&ctx->buf_len,
                                (unsigned char *)in,n);
+                       OPENSSL_assert(ctx->buf_len <= (int)sizeof(ctx->buf));
+                       OPENSSL_assert(ctx->buf_len >= ctx->buf_off);
+                       ret += n;
                        }
                inl-=n;
                in+=n;
@@ -454,8 +476,11 @@ static int b64_write(BIO *b, const char *in, int inl)
                                BIO_copy_next_retry(b);
                                return((ret == 0)?i:ret);
                                }
+                       OPENSSL_assert(i <= n);
                        n-=i;
                        ctx->buf_off+=i;
+                       OPENSSL_assert(ctx->buf_off <= (int)sizeof(ctx->buf));
+                       OPENSSL_assert(ctx->buf_len >= ctx->buf_off);
                        }
                ctx->buf_len=0;
                ctx->buf_off=0;
@@ -486,6 +511,7 @@ static long b64_ctrl(BIO *b, int cmd, long num, void *ptr)
                        ret=BIO_ctrl(b->next_bio,cmd,num,ptr);
                break;
        case BIO_CTRL_WPENDING: /* More to write in buffer */
+               OPENSSL_assert(ctx->buf_len >= ctx->buf_off);
                ret=ctx->buf_len-ctx->buf_off;
                if ((ret == 0) && (ctx->encode != B64_NONE)
                        && (ctx->base64.num != 0))
@@ -494,6 +520,7 @@ static long b64_ctrl(BIO *b, int cmd, long num, void *ptr)
                        ret=BIO_ctrl(b->next_bio,cmd,num,ptr);
                break;
        case BIO_CTRL_PENDING: /* More to read in buffer */
+               OPENSSL_assert(ctx->buf_len >= ctx->buf_off);
                ret=ctx->buf_len-ctx->buf_off;
                if (ret <= 0)
                        ret=BIO_ctrl(b->next_bio,cmd,num,ptr);
@@ -565,3 +592,7 @@ static long b64_callback_ctrl(BIO *b, int cmd, bio_info_cb *fp)
        return(ret);
        }
 
+static int b64_puts(BIO *b, const char *str)
+       {
+       return b64_write(b,str,strlen(str));
+       }