Bug 1515342 - More thorough input checking, r=jcj
All part of applying better discipline throughout.
Differential Revision:
https://phabricator.services.mozilla.com/D33736
--- a/lib/cryptohi/seckey.c
+++ b/lib/cryptohi/seckey.c
@@ -634,16 +634,21 @@ seckey_ExtractPublicKey(const CERTSubjec
rv = SEC_QuickDERDecodeItem(arena, pubk, SECKEY_DHParamKeyTemplate,
&newParms);
if (rv == SECSuccess)
return pubk;
break;
case SEC_OID_ANSIX962_EC_PUBLIC_KEY:
+ /* A basic sanity check on inputs. */
+ if (spki->algorithm.parameters.len == 0 || newOs.len == 0) {
+ PORT_SetError(SEC_ERROR_INPUT_LEN);
+ break;
+ }
pubk->keyType = ecKey;
pubk->u.ec.size = 0;
/* Since PKCS#11 directly takes the DER encoding of EC params
* and public value, we don't need any decoding here.
*/
rv = SECITEM_CopyItem(arena, &pubk->u.ec.DEREncodedParams,
&spki->algorithm.parameters);
--- a/lib/freebl/dh.c
+++ b/lib/freebl/dh.c
@@ -205,17 +205,18 @@ DH_Derive(SECItem *publicValue,
SECItem *derivedSecret,
unsigned int outBytes)
{
mp_int p, Xa, Yb, ZZ, psub1;
mp_err err = MP_OKAY;
unsigned int len = 0;
unsigned int nb;
unsigned char *secret = NULL;
- if (!publicValue || !prime || !privateValue || !derivedSecret) {
+ if (!publicValue || !publicValue->len || !prime || !prime->len ||
+ !privateValue || !privateValue->len || !derivedSecret) {
PORT_SetError(SEC_ERROR_INVALID_ARGS);
return SECFailure;
}
memset(derivedSecret, 0, sizeof *derivedSecret);
MP_DIGITS(&p) = 0;
MP_DIGITS(&Xa) = 0;
MP_DIGITS(&Yb) = 0;
MP_DIGITS(&ZZ) = 0;
--- a/lib/freebl/ec.c
+++ b/lib/freebl/ec.c
@@ -197,18 +197,18 @@ ec_NewKey(ECParams *ecParams, ECPrivateK
mp_err err = MP_OKAY;
int len;
#if EC_DEBUG
printf("ec_NewKey called\n");
#endif
MP_DIGITS(&k) = 0;
- if (!ecParams || !privKey || !privKeyBytes || (privKeyLen < 0) ||
- !ecParams->name) {
+ if (!ecParams || ecParams->name == ECCurve_noName ||
+ !privKey || !privKeyBytes || privKeyLen <= 0) {
PORT_SetError(SEC_ERROR_INVALID_ARGS);
return SECFailure;
}
/* Initialize an arena for the EC key. */
if (!(arena = PORT_NewArena(NSS_FREEBL_DEFAULT_CHUNKSIZE)))
return SECFailure;
@@ -386,17 +386,17 @@ cleanup:
*/
SECStatus
EC_NewKey(ECParams *ecParams, ECPrivateKey **privKey)
{
SECStatus rv = SECFailure;
int len;
unsigned char *privKeyBytes = NULL;
- if (!ecParams) {
+ if (!ecParams || ecParams->name == ECCurve_noName || !privKey) {
PORT_SetError(SEC_ERROR_INVALID_ARGS);
return SECFailure;
}
len = ecParams->order.len;
privKeyBytes = ec_GenerateRandomPrivateKey(ecParams->order.data, len);
if (privKeyBytes == NULL)
goto cleanup;
@@ -425,17 +425,18 @@ SECStatus
EC_ValidatePublicKey(ECParams *ecParams, SECItem *publicValue)
{
mp_int Px, Py;
ECGroup *group = NULL;
SECStatus rv = SECFailure;
mp_err err = MP_OKAY;
int len;
- if (!ecParams || !publicValue || !ecParams->name) {
+ if (!ecParams || ecParams->name == ECCurve_noName ||
+ !publicValue || !publicValue->len) {
PORT_SetError(SEC_ERROR_INVALID_ARGS);
return SECFailure;
}
/* Uses curve specific code for point validation. */
if (ecParams->fieldID.type == ec_field_plain) {
const ECMethod *method = ec_get_method_from_name(ecParams->name);
if (method == NULL || method->validate == NULL) {
@@ -531,18 +532,19 @@ ECDH_Derive(SECItem *publicValue,
SECItem pointQ = { siBuffer, NULL, 0 };
mp_int k; /* to hold the private value */
mp_int cofactor;
mp_err err = MP_OKAY;
#if EC_DEBUG
int i;
#endif
- if (!publicValue || !ecParams || !privateValue || !derivedSecret ||
- !ecParams->name) {
+ if (!publicValue || !publicValue->len ||
+ !ecParams || ecParams->name == ECCurve_noName ||
+ !privateValue || !privateValue->len || !derivedSecret) {
PORT_SetError(SEC_ERROR_INVALID_ARGS);
return SECFailure;
}
/*
* Make sure the point is on the requested curve to avoid
* certain small subgroup attacks.
*/
--- a/lib/util/quickder.c
+++ b/lib/util/quickder.c
@@ -752,16 +752,23 @@ DecodeItem(void* dest,
temp.data++;
temp.len--;
}
}
break;
}
case SEC_ASN1_BIT_STRING: {
+ /* Can't be 8 or more spare bits, or any spare bits
+ * if there are no octets. */
+ if (temp.data[0] >= 8 || (temp.data[0] > 0 && temp.len == 1)) {
+ PORT_SetError(SEC_ERROR_BAD_DER);
+ rv = SECFailure;
+ break;
+ }
/* change the length in the SECItem to be the number
of bits */
temp.len = (temp.len - 1) * 8 - (temp.data[0] & 0x7);
temp.data++;
break;
}
default: {