Skip to content

Commit

Permalink
Fix a failure to NULL a pointer freed on error.
Browse files Browse the repository at this point in the history
Reported by the LibreSSL project as a follow on to CVE-2015-0209

Reviewed-by: Richard Levitte <levitte@openssl.org>
  • Loading branch information
mattcaswell committed Mar 19, 2015
1 parent 367eab2 commit 5e5d53d
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 3 deletions.
12 changes: 11 additions & 1 deletion crypto/asn1/x_x509.c
Expand Up @@ -168,8 +168,14 @@ X509 *d2i_X509_AUX(X509 **a, const unsigned char **pp, long length)
{
const unsigned char *q;
X509 *ret;
int freeret = 0;

/* Save start position */
q = *pp;

if(!a || *a == NULL) {
freeret = 1;
}
ret = d2i_X509(a, pp, length);
/* If certificate unreadable then forget it */
if (!ret)
Expand All @@ -182,7 +188,11 @@ X509 *d2i_X509_AUX(X509 **a, const unsigned char **pp, long length)
goto err;
return ret;
err:
X509_free(ret);
if(freeret) {
X509_free(ret);
if (a)
*a = NULL;
}
return NULL;
}

Expand Down
7 changes: 5 additions & 2 deletions crypto/ec/ec_asn1.c
Expand Up @@ -1226,16 +1226,19 @@ EC_KEY *d2i_ECParameters(EC_KEY **a, const unsigned char **in, long len)
ECerr(EC_F_D2I_ECPARAMETERS, ERR_R_MALLOC_FAILURE);
return NULL;
}
if (a)
*a = ret;
} else
ret = *a;

if (!d2i_ECPKParameters(&ret->group, in, len)) {
ECerr(EC_F_D2I_ECPARAMETERS, ERR_R_EC_LIB);
if (a == NULL || *a != ret)
EC_KEY_free(ret);
return NULL;
}

if (a)
*a = ret;

return ret;
}

Expand Down

1 comment on commit 5e5d53d

@rwessman
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leaving "ret" (line 170) uninitialized leaves open the possibility that a future change that causes a jump to the err label before it is set at line 179. It would be safer to initialize it when it is declared.

Please sign in to comment.