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>
diff --git a/crypto/asn1/tasn_dec.c b/crypto/asn1/tasn_dec.c
index 507a842..d852ad7 100644
--- a/crypto/asn1/tasn_dec.c
+++ b/crypto/asn1/tasn_dec.c
@@ -359,9 +359,9 @@
}
asn1_set_choice_selector(pval, i, it);
- *in = p;
if (asn1_cb && !asn1_cb(ASN1_OP_D2I_POST, pval, it, NULL))
goto auxerr;
+ *in = p;
return 1;
case ASN1_ITYPE_NDEF_SEQUENCE:
@@ -515,9 +515,9 @@
/* Save encoding */
if (!asn1_enc_save(pval, *in, p - *in, it))
goto auxerr;
- *in = p;
if (asn1_cb && !asn1_cb(ASN1_OP_D2I_POST, pval, it, NULL))
goto auxerr;
+ *in = p;
return 1;
default:
diff --git a/crypto/ec/ec_asn1.c b/crypto/ec/ec_asn1.c
index 31d8944..f540256 100644
--- a/crypto/ec/ec_asn1.c
+++ b/crypto/ec/ec_asn1.c
@@ -239,8 +239,9 @@
long len) {
EC_GROUP *group = NULL;
ECPKPARAMETERS *params = NULL;
+ const uint8_t *in = *inp;
- params = d2i_ECPKPARAMETERS(NULL, inp, len);
+ params = d2i_ECPKPARAMETERS(NULL, &in, len);
if (params == NULL) {
OPENSSL_PUT_ERROR(EC, EC_R_D2I_ECPKPARAMETERS_FAILURE);
ECPKPARAMETERS_free(params);
@@ -260,6 +261,7 @@
}
ECPKPARAMETERS_free(params);
+ *inp = in;
return group;
}
@@ -280,12 +282,13 @@
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;
EC_KEY *ret = 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) {
OPENSSL_PUT_ERROR(EC, ERR_R_EC_LIB);
return NULL;
@@ -364,6 +367,7 @@
if (a) {
*a = ret;
}
+ *inp = in;
ok = 1;
err:
diff --git a/crypto/evp/evp_asn1.c b/crypto/evp/evp_asn1.c
index 356c62b..08956e1 100644
--- a/crypto/evp/evp_asn1.c
+++ b/crypto/evp/evp_asn1.c
@@ -83,16 +83,20 @@
goto err;
}
+ const uint8_t *in = *inp;
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) {
- 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) {
goto err;
}
EVP_PKEY_free(ret);
ret = EVP_PKCS82PKEY(p8);
PKCS8_PRIV_KEY_INFO_free(p8);
+ if (ret == NULL) {
+ goto err;
+ }
} else {
OPENSSL_PUT_ERROR(EVP, ERR_R_ASN1_LIB);
goto err;
@@ -102,6 +106,7 @@
if (out != NULL) {
*out = ret;
}
+ *inp = in;
return ret;
err:
@@ -129,7 +134,8 @@
keytype = EVP_PKEY_EC;
} else if (sk_ASN1_TYPE_num(inkey) == 3) {
/* 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;
sk_ASN1_TYPE_pop_free(inkey, ASN1_TYPE_free);
@@ -139,6 +145,11 @@
}
ret = EVP_PKCS82PKEY(p8);
PKCS8_PRIV_KEY_INFO_free(p8);
+ if (ret == NULL) {
+ return NULL;
+ }
+
+ *inp = p;
if (out) {
*out = ret;
}
diff --git a/crypto/x509/x_x509.c b/crypto/x509/x_x509.c
index c975dd3..b8f318a 100644
--- a/crypto/x509/x_x509.c
+++ b/crypto/x509/x_x509.c
@@ -178,22 +178,21 @@
X509 *d2i_X509_AUX(X509 **a, const unsigned char **pp, long length)
{
- const unsigned char *q;
+ const unsigned char *q = *pp;
X509 *ret;
int freeret = 0;
- /* Save start position */
- q = *pp;
-
if (!a || *a == NULL)
freeret = 1;
- ret = d2i_X509(a, pp, length);
+ ret = d2i_X509(a, &q, length);
/* If certificate unreadable then forget it */
if(!ret) return NULL;
/* update length */
- length -= *pp - q;
- if(!length) return ret;
- if(!d2i_X509_CERT_AUX(&ret->aux, pp, length)) goto err;
+ length -= q - *pp;
+ /* Parse auxiliary information if there is any. */
+ if (length > 0 && !d2i_X509_CERT_AUX(&ret->aux, &q, length))
+ goto err;
+ *pp = q;
return ret;
err:
if (freeret)
