diff options
author | Viktor Dukhovni <openssl-users@dukhovni.org> | 2016-05-02 14:46:51 -0400 |
---|---|---|
committer | Viktor Dukhovni <openssl-users@dukhovni.org> | 2016-05-11 01:46:06 -0400 |
commit | fde2257f055f187e8e78542ea6d64ad6c206d10b (patch) | |
tree | 8255a3511f0df776420218852273595f75fd5bcb /crypto/x509/x_x509.c | |
parent | 9b5164ce7788d6985b005e410bb7b53bd553c99e (diff) |
Fix i2d_X509_AUX, update docs and add tests
When *pp is NULL, don't write garbage, return an unexpected pointer
or leak memory on error.
Reviewed-by: Dr. Stephen Henson <steve@openssl.org>
Diffstat (limited to 'crypto/x509/x_x509.c')
-rw-r--r-- | crypto/x509/x_x509.c | 54 |
1 files changed, 52 insertions, 2 deletions
diff --git a/crypto/x509/x_x509.c b/crypto/x509/x_x509.c index 043ab07866..3eba360c47 100644 --- a/crypto/x509/x_x509.c +++ b/crypto/x509/x_x509.c @@ -181,12 +181,26 @@ X509 *d2i_X509_AUX(X509 **a, const unsigned char **pp, long length) return NULL; } -int i2d_X509_AUX(X509 *a, unsigned char **pp) +/* + * Serialize trusted certificate to *pp or just return the required buffer + * length if pp == NULL. We ultimately want to avoid modifying *pp in the + * error path, but that depends on similar hygiene in lower-level functions. + * Here we avoid compounding the problem. + */ +static int i2d_x509_aux_internal(X509 *a, unsigned char **pp) { int length, tmplen; unsigned char *start = pp != NULL ? *pp : NULL; + + OPENSSL_assert(pp == NULL || *pp != NULL); + + /* + * This might perturb *pp on error, but fixing that belongs in i2d_X509() + * not here. It should be that if a == NULL length is zero, but we check + * both just in case. + */ length = i2d_X509(a, pp); - if (length < 0 || a == NULL) + if (length <= 0 || a == NULL) return length; tmplen = i2d_X509_CERT_AUX(a->aux, pp); @@ -200,6 +214,42 @@ int i2d_X509_AUX(X509 *a, unsigned char **pp) return length; } +/* + * Serialize trusted certificate to *pp, or just return the required buffer + * length if pp == NULL. + * + * When pp is not NULL, but *pp == NULL, we allocate the buffer, but since + * we're writing two ASN.1 objects back to back, we can't have i2d_X509() do + * the allocation, nor can we allow i2d_X509_CERT_AUX() to increment the + * allocated buffer. + */ +int i2d_X509_AUX(X509 *a, unsigned char **pp) +{ + int length; + unsigned char *tmp; + + /* Buffer provided by caller */ + if (pp == NULL || *pp != NULL) + return i2d_x509_aux_internal(a, pp); + + /* Obtain the combined length */ + if ((length = i2d_x509_aux_internal(a, NULL)) <= 0) + return length; + + /* Allocate requisite combined storage */ + *pp = tmp = OPENSSL_malloc(length); + if (tmp == NULL) + return -1; /* Push error onto error stack? */ + + /* Encode, but keep *pp at the originally malloced pointer */ + length = i2d_x509_aux_internal(a, &tmp); + if (length <= 0) { + OPENSSL_free(*pp); + *pp = NULL; + } + return length; +} + int i2d_re_X509_tbs(X509 *x, unsigned char **pp) { x->cert_info.enc.modified = 1; |