Re: [PATCH 1/7] If EVP_DecryptInit_ex() returns NULL, j is incremented by a random amount in PEM_do_header()

[Joel Sing]

On Sun, 1 Jun 2014, Brent Cook wrote:
> clang warning:
> pem/pem_lib.c:472:6: error: variable 'i' is used uninitialized whenever
> 'if' condition is false [-Werror,-Wsometimes-uninitialized]
>         if (o)
>             ^
> pem/pem_lib.c:479:7: note: uninitialized use occurs here
>         j += i;
>              ^
> pem/pem_lib.c:472:2: note: remove the 'if' if its condition is always true
>         if (o)
>         ^~~~~~
> pem/pem_lib.c:446:7: note: initialize the variable 'i' to silence this
> warning int i, j, o, klen;
> ---
>  src/crypto/pem/pem_lib.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/src/crypto/pem/pem_lib.c b/src/crypto/pem/pem_lib.c
> index 945262f..92c3dc4 100644
> --- a/src/crypto/pem/pem_lib.c
> +++ b/src/crypto/pem/pem_lib.c
> @@ -454,6 +454,7 @@ PEM_do_header(EVP_CIPHER_INFO *cipher, unsigned char
> *data, long *plen, (unsigned char *)buf, klen, 1, key, NULL))
>               return 0;
>
> +     i = 0;
>       j = (int)len;
>       EVP_CIPHER_CTX_init(&ctx);
>       o = EVP_DecryptInit_ex(&ctx, cipher->cipher, NULL, key,

This is a non-issue since the value of j is unused in the !o case. That
said, I've just commited the following diff actually fixes the code,
rather than just addressing the uninitialised variable:

Index: pem_lib.c
===================================================================
RCS file: /cvs/src/lib/libssl/src/crypto/pem/pem_lib.c,v
retrieving revision 1.23
diff -u -p -r1.23 pem_lib.c
--- pem_lib.c   26 Apr 2014 18:56:38 -0000      1.23
+++ pem_lib.c   29 May 2014 15:39:26 -0000
@@ -476,12 +476,11 @@ PEM_do_header(EVP_CIPHER_INFO *cipher, u
        EVP_CIPHER_CTX_cleanup(&ctx);
        OPENSSL_cleanse((char *)buf, sizeof(buf));
        OPENSSL_cleanse((char *)key, sizeof(key));
-       j += i;
        if (!o) {
                PEMerr(PEM_F_PEM_DO_HEADER, PEM_R_BAD_DECRYPT);
                return (0);
        }
-       *plen = j;
+       *plen = j + i;
        return (1);
 }
 


-- 

    "Action without study is fatal. Study without action is futile."
        -- Mary Ritter Beard