PKCS7_decrypt behavior

[Arnaud De Timmerman]

hi all,

i'm refering to this post i think it's better to write here
there might be a memory leak in ./crypto/pkcs7/pk7_smime.c
at the beginning i thought i was a fool, but i've seen that the same error was
elsewhere in the code (thanks to Changes between 0.9.6h and 0.9.7).

Geoff says :
********************
  *) Fix a memory leak in 'sk_dup()' in the case reallocation fails. (Also
     tidy up some unnecessarily weird code in 'sk_new()').
     [Geoff, reported by Diego Tartara <[EMAIL PROTECTED]>]
********************

the code in openssl 0.9.6g is :
********************
STACK *sk_dup(STACK *sk)
     {
     STACK *ret;
     char **s;

     if ((ret=sk_new(sk->comp)) == NULL) goto err;
     s=(char **)OPENSSL_realloc((char *)ret->data,
          (unsigned int)sizeof(char *)*sk->num_alloc);
     if (s == NULL) goto err;
     ret->data=s;

     ret->num=sk->num;
     memcpy(ret->data,sk->data,sizeof(char *)*sk->num);
     ret->sorted=sk->sorted;
     ret->num_alloc=sk->num_alloc;
     ret->comp=sk->comp;
     return(ret);
err:
     return(NULL);
     }
********************

the new one is
********************
STACK *sk_dup(STACK *sk)
     {
     STACK *ret;
     char **s;

     if ((ret=sk_new(sk->comp)) == NULL) goto err;
     s=(char **)OPENSSL_realloc((char *)ret->data,
          (unsigned int)sizeof(char *)*sk->num_alloc);
     if (s == NULL) goto err;
     ret->data=s;

     ret->num=sk->num;
     memcpy(ret->data,sk->data,sizeof(char *)*sk->num);
     ret->sorted=sk->sorted;
     ret->num_alloc=sk->num_alloc;
     ret->comp=sk->comp;
     return(ret);
err:
     if(ret)
          sk_free(ret);
     return(NULL);
     }
********************

i think the same thing occurs in PKCS7_decrypt, the code is :
********************
int PKCS7_decrypt(PKCS7 *p7, EVP_PKEY *pkey, X509 *cert, BIO *data, int flags)
{
     BIO *tmpmem;
     int ret, i;
     char buf[4096];

     if(!p7) {
          PKCS7err(PKCS7_F_PKCS7_DECRYPT,PKCS7_R_INVALID_NULL_POINTER);
          return 0;
     }

     if(!PKCS7_type_is_enveloped(p7)) {
          PKCS7err(PKCS7_F_PKCS7_DECRYPT,PKCS7_R_WRONG_CONTENT_TYPE);
          return 0;
     }

     if(!X509_check_private_key(cert, pkey)) {
          PKCS7err(PKCS7_F_PKCS7_DECRYPT,
                    PKCS7_R_PRIVATE_KEY_DOES_NOT_MATCH_CERTIFICATE);
          return 0;
     }

     if(!(tmpmem = PKCS7_dataDecode(p7, pkey, NULL, cert))) {
          PKCS7err(PKCS7_F_PKCS7_DECRYPT, PKCS7_R_DECRYPT_ERROR);
          return 0;
     }

     if (flags & PKCS7_TEXT) {
          BIO *tmpbuf, *bread;
          /* Encrypt BIOs can't do BIO_gets() so add a buffer BIO */
          if(!(tmpbuf = BIO_new(BIO_f_buffer()))) {
               PKCS7err(PKCS7_F_PKCS7_DECRYPT, ERR_R_MALLOC_FAILURE);
               return 0;
          }
          if(!(bread = BIO_push(tmpbuf, tmpmem))) {
               PKCS7err(PKCS7_F_PKCS7_DECRYPT, ERR_R_MALLOC_FAILURE);
               return 0;
          }
          ret = SMIME_text(bread, data);
          BIO_free_all(bread);
          return ret;
     } else {
          for(;;) {
               i = BIO_read(tmpmem, buf, sizeof(buf));
               if(i <= 0) break;
               BIO_write(data, buf, i);
          }
          BIO_free_all(tmpmem);
          return 1;
     }
}
********************

to the best of my knowledge "tmpmem" (and maybe "tmpbuf") can still be allocated
when the function ends)
please tell me what you think

bye


______________________________________________________________________
OpenSSL Project                                 http://www.openssl.org
Development Mailing List                       [EMAIL PROTECTED]
Automated List Manager                           [EMAIL PROTECTED]