X509_TRUST_add() and X509_PURPOSE_add() leak memory or corrupt existing
entries when they fail (ie. when memory is exhausted, or the name /
sname argument to BUF_strdup is NULL.)
This seems like an unlikely error to hit, but we may as well handle it
correctly.
Brendan
Index: libssl/src/crypto/x509/x509_trs.c
===================================================================
--- libssl.orig/src/crypto/x509/x509_trs.c
+++ libssl/src/crypto/x509/x509_trs.c
@@ -174,6 +174,7 @@ int
X509_TRUST_add(int id, int flags, int (*ck)(X509_TRUST *, X509 *, int),
char *name, int arg1, void *arg2)
{
+ char *tmpname = NULL;
int idx;
X509_TRUST *trtmp;
@@ -187,20 +188,22 @@ X509_TRUST_add(int id, int flags, int (*
if (idx == -1) {
if (!(trtmp = malloc(sizeof(X509_TRUST)))) {
X509err(X509_F_X509_TRUST_ADD, ERR_R_MALLOC_FAILURE);
- return 0;
+ goto err;
}
trtmp->flags = X509_TRUST_DYNAMIC;
} else
trtmp = X509_TRUST_get0(idx);
- /* free existing name if dynamic */
- if (trtmp->flags & X509_TRUST_DYNAMIC_NAME)
- free(trtmp->name);
/* dup supplied name */
- if (!(trtmp->name = BUF_strdup(name))) {
+ if (!(tmpname = BUF_strdup(name))) {
X509err(X509_F_X509_TRUST_ADD, ERR_R_MALLOC_FAILURE);
- return 0;
+ goto err;
}
+ /* free existing name if dynamic */
+ if (trtmp->flags & X509_TRUST_DYNAMIC_NAME)
+ free(trtmp->name);
+ trtmp->name = tmpname;
+
/* Keep the dynamic flag of existing entry */
trtmp->flags &= X509_TRUST_DYNAMIC;
/* Set all other flags */
@@ -215,14 +218,20 @@ X509_TRUST_add(int id, int flags, int (*
if (idx == -1) {
if (!trtable && !(trtable = sk_X509_TRUST_new(tr_cmp))) {
X509err(X509_F_X509_TRUST_ADD, ERR_R_MALLOC_FAILURE);
- return 0;
+ goto err;
}
if (!sk_X509_TRUST_push(trtable, trtmp)) {
X509err(X509_F_X509_TRUST_ADD, ERR_R_MALLOC_FAILURE);
- return 0;
+ goto err;
}
}
return 1;
+
+err:
+ free(tmpname);
+ if (idx == -1)
+ free(trtmp);
+ return 0;
}
static void
Index: libssl/src/crypto/x509v3/v3_purp.c
===================================================================
--- libssl.orig/src/crypto/x509v3/v3_purp.c
+++ libssl/src/crypto/x509v3/v3_purp.c
@@ -197,6 +197,7 @@ X509_PURPOSE_add(int id, int trust, int
int (*ck)(const X509_PURPOSE *, const X509 *, int), char *name,
char *sname, void *arg)
{
+ char *tmpname = NULL, *tmpsname = NULL;
int idx;
X509_PURPOSE *ptmp;
@@ -211,24 +212,27 @@ X509_PURPOSE_add(int id, int trust, int
if (!(ptmp = malloc(sizeof(X509_PURPOSE)))) {
X509V3err(X509V3_F_X509_PURPOSE_ADD,
ERR_R_MALLOC_FAILURE);
- return 0;
+ goto err;
}
ptmp->flags = X509_PURPOSE_DYNAMIC;
} else
ptmp = X509_PURPOSE_get0(idx);
+ /* dup supplied name */
+ tmpname = BUF_strdup(name);
+ tmpsname = BUF_strdup(sname);
+ if (!tmpname || !tmpsname) {
+ X509V3err(X509V3_F_X509_PURPOSE_ADD, ERR_R_MALLOC_FAILURE);
+ goto err;
+ }
/* free existing name if dynamic */
if (ptmp->flags & X509_PURPOSE_DYNAMIC_NAME) {
free(ptmp->name);
free(ptmp->sname);
}
- /* dup supplied name */
- ptmp->name = BUF_strdup(name);
- ptmp->sname = BUF_strdup(sname);
- if (!ptmp->name || !ptmp->sname) {
- X509V3err(X509V3_F_X509_PURPOSE_ADD, ERR_R_MALLOC_FAILURE);
- return 0;
- }
+ ptmp->name = tmpname;
+ ptmp->sname = tmpsname;
+
/* Keep the dynamic flag of existing entry */
ptmp->flags &= X509_PURPOSE_DYNAMIC;
/* Set all other flags */
@@ -244,15 +248,22 @@ X509_PURPOSE_add(int id, int trust, int
if (!xptable && !(xptable = sk_X509_PURPOSE_new(xp_cmp))) {
X509V3err(X509V3_F_X509_PURPOSE_ADD,
ERR_R_MALLOC_FAILURE);
- return 0;
+ goto err;
}
if (!sk_X509_PURPOSE_push(xptable, ptmp)) {
X509V3err(X509V3_F_X509_PURPOSE_ADD,
ERR_R_MALLOC_FAILURE);
- return 0;
+ goto err;
}
}
return 1;
+
+err:
+ free(tmpname);
+ free(tmpsname);
+ if (idx == -1)
+ free(ptmp);
+ return 0;
}
static void
