From fa8b0e673622deb4d67806fb48a9b67876601c04 Mon Sep 17 00:00:00 2001 From: Cameron Kaiser Date: Thu, 4 Jul 2019 10:08:32 -0700 Subject: [PATCH] #559: M1547757 M1539219 M1548306 M1558548 --- dom/indexedDB/ActorsChild.cpp | 7 ++++++- dom/media/MediaResource.cpp | 2 +- netwerk/base/nsStandardURL.cpp | 2 +- security/nss/lib/cryptohi/seckey.c | 6 ++++++ security/nss/lib/freebl/dh.c | 3 ++- security/nss/lib/freebl/ec.c | 13 ++++++++----- security/nss/lib/freebl/mpi/mpi.c | 5 ++++- security/nss/lib/pk11wrap/pk11akey.c | 21 ++++++++++++++------- security/nss/lib/pk11wrap/pk11cert.c | 4 +++- security/nss/lib/pk11wrap/pk11pk12.c | 1 + security/nss/lib/softoken/legacydb/lgattr.c | 6 +++--- security/nss/lib/softoken/pkcs11c.c | 2 +- security/nss/lib/util/quickder.c | 11 +++++++++-- 13 files changed, 59 insertions(+), 24 deletions(-) diff --git a/dom/indexedDB/ActorsChild.cpp b/dom/indexedDB/ActorsChild.cpp index 443ad8b02..e70c10d0a 100644 --- a/dom/indexedDB/ActorsChild.cpp +++ b/dom/indexedDB/ActorsChild.cpp @@ -2214,9 +2214,14 @@ BackgroundVersionChangeTransactionChild::RecvComplete(const nsresult& aResult) database->Close(); } + RefPtr request = mOpenDBRequest; + MOZ_ASSERT(request); + mTransaction->FireCompleteOrAbortEvents(aResult); - mOpenDBRequest->SetTransaction(nullptr); + request->SetTransaction(nullptr); + request = nullptr; + mOpenDBRequest = nullptr; NoteComplete(); diff --git a/dom/media/MediaResource.cpp b/dom/media/MediaResource.cpp index ed40054e1..7ed380649 100644 --- a/dom/media/MediaResource.cpp +++ b/dom/media/MediaResource.cpp @@ -1567,7 +1567,7 @@ void BaseMediaResource::SetLoadInBackground(bool aLoadInBackground) { NS_WARNING("Null owner in MediaResource::SetLoadInBackground()"); return; } - dom::HTMLMediaElement* element = owner->GetMediaElement(); + RefPtr element = owner->GetMediaElement(); if (!element) { NS_WARNING("Null element in MediaResource::SetLoadInBackground()"); return; diff --git a/netwerk/base/nsStandardURL.cpp b/netwerk/base/nsStandardURL.cpp index e88af0fd2..2129966d1 100644 --- a/netwerk/base/nsStandardURL.cpp +++ b/netwerk/base/nsStandardURL.cpp @@ -447,7 +447,7 @@ nsStandardURL::ValidIPv6orHostname(const char *host, uint32_t length) } const char *end = host + length; - if (end != net_FindCharInSet(host, end, "\t\n\v\f\r #/:?@[\\]")) { + if (end != net_FindCharInSet(host, end, "\t\n\v\f\r ^#/:?@[\\]")) { // We still allow % because it is in the ID of addons. // Any percent encoded ASCII characters that are not allowed in the // hostname are not percent decoded, and will be parsed just fine. diff --git a/security/nss/lib/cryptohi/seckey.c b/security/nss/lib/cryptohi/seckey.c index 1fcd4087f..aebe96986 100644 --- a/security/nss/lib/cryptohi/seckey.c +++ b/security/nss/lib/cryptohi/seckey.c @@ -618,6 +618,12 @@ seckey_ExtractPublicKey(const CERTSubjectPublicKeyInfo *spki) 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; diff --git a/security/nss/lib/freebl/dh.c b/security/nss/lib/freebl/dh.c index 66c110134..cd9a7ac60 100644 --- a/security/nss/lib/freebl/dh.c +++ b/security/nss/lib/freebl/dh.c @@ -208,7 +208,8 @@ DH_Derive(SECItem *publicValue, 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; } diff --git a/security/nss/lib/freebl/ec.c b/security/nss/lib/freebl/ec.c index 4435f91ea..1c21551ed 100644 --- a/security/nss/lib/freebl/ec.c +++ b/security/nss/lib/freebl/ec.c @@ -215,7 +215,8 @@ ec_NewKey(ECParams *ecParams, ECPrivateKey **privKey, #endif MP_DIGITS(&k) = 0; - if (!ecParams || !privKey || !privKeyBytes || (privKeyLen < 0)) { + if (!ecParams || ecParams->name == ECCurve_noName || + !privKey || !privKeyBytes || privKeyLen <= 0) { PORT_SetError(SEC_ERROR_INVALID_ARGS); return SECFailure; } @@ -395,7 +396,7 @@ EC_NewKey(ECParams *ecParams, ECPrivateKey **privKey) int len; unsigned char *privKeyBytes = NULL; - if (!ecParams) { + if (!ecParams || ecParams->name == ECCurve_noName || !privKey) { PORT_SetError(SEC_ERROR_INVALID_ARGS); return SECFailure; } @@ -437,7 +438,8 @@ EC_ValidatePublicKey(ECParams *ecParams, SECItem *publicValue) mp_err err = MP_OKAY; int len; - if (!ecParams || !publicValue) { + if (!ecParams || ecParams->name == ECCurve_noName || + !publicValue || !publicValue->len) { PORT_SetError(SEC_ERROR_INVALID_ARGS); return SECFailure; } @@ -537,8 +539,9 @@ ECDH_Derive(SECItem *publicValue, int i; #endif - if (!publicValue || !ecParams || !privateValue || - !derivedSecret) { + if (!publicValue || !publicValue->len || + !ecParams || ecParams->name == ECCurve_noName || + !privateValue || !privateValue->len || !derivedSecret) { PORT_SetError(SEC_ERROR_INVALID_ARGS); return SECFailure; } diff --git a/security/nss/lib/freebl/mpi/mpi.c b/security/nss/lib/freebl/mpi/mpi.c index 4bda09715..ac9159e2e 100644 --- a/security/nss/lib/freebl/mpi/mpi.c +++ b/security/nss/lib/freebl/mpi/mpi.c @@ -2104,7 +2104,10 @@ mp_err s_mp_almost_inverse(const mp_int *a, const mp_int *p, mp_int *c) } } if (res >= 0) { - while (MP_SIGN(c) != MP_ZPOS) { + if (mp_cmp_mag(c, p) >= 0) { + MP_CHECKOK(mp_div(c, p, NULL, c)); + } + if (MP_SIGN(c) != MP_ZPOS) { MP_CHECKOK( mp_add(c, p, c) ); } res = k; diff --git a/security/nss/lib/pk11wrap/pk11akey.c b/security/nss/lib/pk11wrap/pk11akey.c index b0604de3a..9fa7c9d89 100644 --- a/security/nss/lib/pk11wrap/pk11akey.c +++ b/security/nss/lib/pk11wrap/pk11akey.c @@ -164,7 +164,6 @@ PK11_ImportPublicKey(PK11SlotInfo *slot, SECKEYPublicKey *pubKey, keyType = CKK_EC; PK11_SETATTRS(attrs, CKA_VERIFY, &cktrue, sizeof(CK_BBOOL));attrs++; PK11_SETATTRS(attrs, CKA_DERIVE, &cktrue, sizeof(CK_BBOOL));attrs++; - signedattr = attrs; PK11_SETATTRS(attrs, CKA_EC_PARAMS, pubKey->u.ec.DEREncodedParams.data, pubKey->u.ec.DEREncodedParams.len); attrs++; @@ -195,10 +194,14 @@ PK11_ImportPublicKey(PK11SlotInfo *slot, SECKEYPublicKey *pubKey, } templateCount = attrs - theTemplate; - signedcount = attrs - signedattr; PORT_Assert(templateCount <= (sizeof(theTemplate)/sizeof(CK_ATTRIBUTE))); - for (attrs=signedattr; signedcount; attrs++, signedcount--) { - pk11_SignedToUnsigned(attrs); + + if (pubKey->keyType != ecKey) { + PORT_Assert(signedattr); + signedcount = attrs - signedattr; + for (attrs = signedattr; signedcount; attrs++, signedcount--) { + pk11_SignedToUnsigned(attrs); + } } rv = PK11_CreateNewObject(slot, CK_INVALID_SESSION, theTemplate, templateCount, isToken, &objectID); @@ -956,9 +959,13 @@ pk11_loadPrivKeyWithFlags(PK11SlotInfo *slot,SECKEYPrivateKey *privKey, &cktrue, &ckfalse); /* Not everyone can handle zero padded key values, give - * them the raw data as unsigned */ - for (ap=attrs; extra_count; ap++, extra_count--) { - pk11_SignedToUnsigned(ap); + * them the raw data as unsigned. The exception is EC, + * where the values are encoded or zero-preserving + * per-RFC5915 */ + if (privKey->keyType != ecKey) { + for (ap = attrs; extra_count; ap++, extra_count--) { + pk11_SignedToUnsigned(ap); + } } /* now Store the puppies */ diff --git a/security/nss/lib/pk11wrap/pk11cert.c b/security/nss/lib/pk11wrap/pk11cert.c index e29b4e212..159844880 100644 --- a/security/nss/lib/pk11wrap/pk11cert.c +++ b/security/nss/lib/pk11wrap/pk11cert.c @@ -172,7 +172,9 @@ PK11_IsUserCert(PK11SlotInfo *slot, CERTCertificate *cert, SECKEY_DestroyPublicKey(pubKey); return PR_FALSE; } - pk11_SignedToUnsigned(&theTemplate); + if (pubKey->keyType != ecKey) { + pk11_SignedToUnsigned(&theTemplate); + } if (pk11_FindObjectByTemplate(slot,&theTemplate,1) != CK_INVALID_HANDLE) { SECKEY_DestroyPublicKey(pubKey); return PR_TRUE; diff --git a/security/nss/lib/pk11wrap/pk11pk12.c b/security/nss/lib/pk11wrap/pk11pk12.c index e5a0a21cf..a3176b2ee 100644 --- a/security/nss/lib/pk11wrap/pk11pk12.c +++ b/security/nss/lib/pk11wrap/pk11pk12.c @@ -282,6 +282,7 @@ PK11_ImportAndReturnPrivateKey(PK11SlotInfo *slot, SECKEYRawPrivateKey *lpk, PK11_SETATTRS(attrs, CKA_PRIVATE, isPrivate ? &cktrue : &ckfalse, sizeof(CK_BBOOL) ); attrs++; + PORT_Assert(lpk->keyType != ecKey); /* see bug 1558548 if this is needed */ switch (lpk->keyType) { case rsaKey: keyType = CKK_RSA; diff --git a/security/nss/lib/softoken/legacydb/lgattr.c b/security/nss/lib/softoken/legacydb/lgattr.c index 429ef8726..b9ff13ef9 100644 --- a/security/nss/lib/softoken/legacydb/lgattr.c +++ b/security/nss/lib/softoken/legacydb/lgattr.c @@ -960,9 +960,9 @@ lg_FindECPrivateKeyAttribute(NSSLOWKEYPrivateKey *key, CK_ATTRIBUTE_TYPE type, case CKA_UNWRAP: return LG_CLONE_ATTR(attribute,type,lg_StaticFalseAttr); case CKA_VALUE: - return lg_CopyPrivAttrSigned(attribute, type, - key->u.ec.privateValue.data, - key->u.ec.privateValue.len, sdbpw); + return lg_CopyPrivAttribute(attribute, type, + key->u.ec.privateValue.data, + key->u.ec.privateValue.len, sdbpw); case CKA_EC_PARAMS: return lg_CopyAttributeSigned(attribute, type, key->u.ec.ecParams.DEREncoding.data, diff --git a/security/nss/lib/softoken/pkcs11c.c b/security/nss/lib/softoken/pkcs11c.c index 8755f24c3..90d35bffc 100644 --- a/security/nss/lib/softoken/pkcs11c.c +++ b/security/nss/lib/softoken/pkcs11c.c @@ -6977,7 +6977,7 @@ key_and_mac_derive_fail: rv = ECDH_Derive(&ecPoint, &privKey->u.ec.ecParams, &ecScalar, withCofactor, &tmp); - PORT_Free(ecScalar.data); + PORT_ZFree(ecScalar.data, ecScalar.len); ecScalar.data = NULL; if (privKey != sourceKey->objectInfo) { nsslowkey_DestroyPrivateKey(privKey); diff --git a/security/nss/lib/util/quickder.c b/security/nss/lib/util/quickder.c index fe72b293a..40e38c518 100644 --- a/security/nss/lib/util/quickder.c +++ b/security/nss/lib/util/quickder.c @@ -870,8 +870,15 @@ static SECStatus DecodeItem(void* dest, break; } - case SEC_ASN1_BIT_STRING: - { + 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);