d2i: don't update input pointer on failure

(Imported from upstream's 728bcd59d3d41e152aead0d15acc51a8958536d3.)

Actually this one was reported by us, but the commit message doesn't
mention this.

This is slightly modified from upstream's version to fix some problems
noticed in import. Specifically one of d2i_X509_AUX's success paths is
bust and d2i_PrivateKey still updates on one error path. Resolve the
latter by changing both it and d2i_AutoPrivateKey to explicitly hit the
error path on ret == NULL. This lets us remove the NULL check in
d2i_AutoPrivateKey.

We'll want to report the problems back upstream.

Change-Id: Ifcfc965ca6d5ec0a08ac154854bd351cafbaba25
Reviewed-on: https://boringssl-review.googlesource.com/5948
Reviewed-by: Adam Langley <agl@google.com>
This commit is contained in:
David Benjamin 2015-09-19 14:13:50 -04:00 committed by Adam Langley
parent 97a33939a3
commit 15e4deb165
4 changed files with 30 additions and 16 deletions

View File

@ -359,9 +359,9 @@ int ASN1_item_ex_d2i(ASN1_VALUE **pval, const unsigned char **in, long len,
} }
asn1_set_choice_selector(pval, i, it); asn1_set_choice_selector(pval, i, it);
*in = p;
if (asn1_cb && !asn1_cb(ASN1_OP_D2I_POST, pval, it, NULL)) if (asn1_cb && !asn1_cb(ASN1_OP_D2I_POST, pval, it, NULL))
goto auxerr; goto auxerr;
*in = p;
return 1; return 1;
case ASN1_ITYPE_NDEF_SEQUENCE: case ASN1_ITYPE_NDEF_SEQUENCE:
@ -515,9 +515,9 @@ int ASN1_item_ex_d2i(ASN1_VALUE **pval, const unsigned char **in, long len,
/* Save encoding */ /* Save encoding */
if (!asn1_enc_save(pval, *in, p - *in, it)) if (!asn1_enc_save(pval, *in, p - *in, it))
goto auxerr; goto auxerr;
*in = p;
if (asn1_cb && !asn1_cb(ASN1_OP_D2I_POST, pval, it, NULL)) if (asn1_cb && !asn1_cb(ASN1_OP_D2I_POST, pval, it, NULL))
goto auxerr; goto auxerr;
*in = p;
return 1; return 1;
default: default:

View File

@ -239,8 +239,9 @@ static EC_GROUP *d2i_ECPKParameters(EC_GROUP **groupp, const uint8_t **inp,
long len) { long len) {
EC_GROUP *group = NULL; EC_GROUP *group = NULL;
ECPKPARAMETERS *params = NULL; ECPKPARAMETERS *params = NULL;
const uint8_t *in = *inp;
params = d2i_ECPKPARAMETERS(NULL, inp, len); params = d2i_ECPKPARAMETERS(NULL, &in, len);
if (params == NULL) { if (params == NULL) {
OPENSSL_PUT_ERROR(EC, EC_R_D2I_ECPKPARAMETERS_FAILURE); OPENSSL_PUT_ERROR(EC, EC_R_D2I_ECPKPARAMETERS_FAILURE);
ECPKPARAMETERS_free(params); ECPKPARAMETERS_free(params);
@ -260,6 +261,7 @@ static EC_GROUP *d2i_ECPKParameters(EC_GROUP **groupp, const uint8_t **inp,
} }
ECPKPARAMETERS_free(params); ECPKPARAMETERS_free(params);
*inp = in;
return group; return group;
} }
@ -280,12 +282,13 @@ static int i2d_ECPKParameters(const EC_GROUP *group, uint8_t **outp) {
return ret; return ret;
} }
EC_KEY *d2i_ECPrivateKey(EC_KEY **a, const uint8_t **in, long len) { EC_KEY *d2i_ECPrivateKey(EC_KEY **a, const uint8_t **inp, long len) {
int ok = 0; int ok = 0;
EC_KEY *ret = NULL; EC_KEY *ret = NULL;
EC_PRIVATEKEY *priv_key = NULL; EC_PRIVATEKEY *priv_key = NULL;
priv_key = d2i_EC_PRIVATEKEY(NULL, in, len); const uint8_t *in = *inp;
priv_key = d2i_EC_PRIVATEKEY(NULL, &in, len);
if (priv_key == NULL) { if (priv_key == NULL) {
OPENSSL_PUT_ERROR(EC, ERR_R_EC_LIB); OPENSSL_PUT_ERROR(EC, ERR_R_EC_LIB);
return NULL; return NULL;
@ -364,6 +367,7 @@ EC_KEY *d2i_ECPrivateKey(EC_KEY **a, const uint8_t **in, long len) {
if (a) { if (a) {
*a = ret; *a = ret;
} }
*inp = in;
ok = 1; ok = 1;
err: err:

View File

@ -83,16 +83,20 @@ EVP_PKEY *d2i_PrivateKey(int type, EVP_PKEY **out, const uint8_t **inp,
goto err; goto err;
} }
const uint8_t *in = *inp;
if (!ret->ameth->old_priv_decode || if (!ret->ameth->old_priv_decode ||
!ret->ameth->old_priv_decode(ret, inp, len)) { !ret->ameth->old_priv_decode(ret, &in, len)) {
if (ret->ameth->priv_decode) { if (ret->ameth->priv_decode) {
PKCS8_PRIV_KEY_INFO *p8 = d2i_PKCS8_PRIV_KEY_INFO(NULL, inp, len); PKCS8_PRIV_KEY_INFO *p8 = d2i_PKCS8_PRIV_KEY_INFO(NULL, &in, len);
if (!p8) { if (!p8) {
goto err; goto err;
} }
EVP_PKEY_free(ret); EVP_PKEY_free(ret);
ret = EVP_PKCS82PKEY(p8); ret = EVP_PKCS82PKEY(p8);
PKCS8_PRIV_KEY_INFO_free(p8); PKCS8_PRIV_KEY_INFO_free(p8);
if (ret == NULL) {
goto err;
}
} else { } else {
OPENSSL_PUT_ERROR(EVP, ERR_R_ASN1_LIB); OPENSSL_PUT_ERROR(EVP, ERR_R_ASN1_LIB);
goto err; goto err;
@ -102,6 +106,7 @@ EVP_PKEY *d2i_PrivateKey(int type, EVP_PKEY **out, const uint8_t **inp,
if (out != NULL) { if (out != NULL) {
*out = ret; *out = ret;
} }
*inp = in;
return ret; return ret;
err: err:
@ -129,7 +134,8 @@ EVP_PKEY *d2i_AutoPrivateKey(EVP_PKEY **out, const uint8_t **inp, long len) {
keytype = EVP_PKEY_EC; keytype = EVP_PKEY_EC;
} else if (sk_ASN1_TYPE_num(inkey) == 3) { } else if (sk_ASN1_TYPE_num(inkey) == 3) {
/* This seems to be PKCS8, not traditional format */ /* This seems to be PKCS8, not traditional format */
PKCS8_PRIV_KEY_INFO *p8 = d2i_PKCS8_PRIV_KEY_INFO(NULL, inp, len); p = *inp;
PKCS8_PRIV_KEY_INFO *p8 = d2i_PKCS8_PRIV_KEY_INFO(NULL, &p, len);
EVP_PKEY *ret; EVP_PKEY *ret;
sk_ASN1_TYPE_pop_free(inkey, ASN1_TYPE_free); sk_ASN1_TYPE_pop_free(inkey, ASN1_TYPE_free);
@ -139,6 +145,11 @@ EVP_PKEY *d2i_AutoPrivateKey(EVP_PKEY **out, const uint8_t **inp, long len) {
} }
ret = EVP_PKCS82PKEY(p8); ret = EVP_PKCS82PKEY(p8);
PKCS8_PRIV_KEY_INFO_free(p8); PKCS8_PRIV_KEY_INFO_free(p8);
if (ret == NULL) {
return NULL;
}
*inp = p;
if (out) { if (out) {
*out = ret; *out = ret;
} }

View File

@ -178,22 +178,21 @@ void *X509_get_ex_data(X509 *r, int idx)
X509 *d2i_X509_AUX(X509 **a, const unsigned char **pp, long length) X509 *d2i_X509_AUX(X509 **a, const unsigned char **pp, long length)
{ {
const unsigned char *q; const unsigned char *q = *pp;
X509 *ret; X509 *ret;
int freeret = 0; int freeret = 0;
/* Save start position */
q = *pp;
if (!a || *a == NULL) if (!a || *a == NULL)
freeret = 1; freeret = 1;
ret = d2i_X509(a, pp, length); ret = d2i_X509(a, &q, length);
/* If certificate unreadable then forget it */ /* If certificate unreadable then forget it */
if(!ret) return NULL; if(!ret) return NULL;
/* update length */ /* update length */
length -= *pp - q; length -= q - *pp;
if(!length) return ret; /* Parse auxiliary information if there is any. */
if(!d2i_X509_CERT_AUX(&ret->aux, pp, length)) goto err; if (length > 0 && !d2i_X509_CERT_AUX(&ret->aux, &q, length))
goto err;
*pp = q;
return ret; return ret;
err: err:
if (freeret) if (freeret)