3d912a
# HG changeset patch
3d912a
# User Martin Thomson <mt@lowentropy.net>
3d912a
# Date 1560498951 0
3d912a
#      Fri Jun 14 07:55:51 2019 +0000
3d912a
# Branch NSS_3_44_BRANCH
3d912a
# Node ID fb9932d6e083322e7b5dfcd3d6e67477e0bb075a
3d912a
# Parent  876bca2723a1f969422edc93e7504420d8331d3c
3d912a
Bug 1515342 - More thorough input checking, r=jcj
3d912a
3d912a
All part of applying better discipline throughout.
3d912a
3d912a
Differential Revision: https://phabricator.services.mozilla.com/D33736
3d912a
3d912a
diff --git a/lib/cryptohi/seckey.c b/lib/cryptohi/seckey.c
3d912a
--- a/lib/cryptohi/seckey.c
3d912a
+++ b/lib/cryptohi/seckey.c
3d912a
@@ -639,6 +639,11 @@ seckey_ExtractPublicKey(const CERTSubjec
3d912a
                     return pubk;
3d912a
                 break;
3d912a
             case SEC_OID_ANSIX962_EC_PUBLIC_KEY:
3d912a
+                /* A basic sanity check on inputs. */
3d912a
+                if (spki->algorithm.parameters.len == 0 || newOs.len == 0) {
3d912a
+                    PORT_SetError(SEC_ERROR_INPUT_LEN);
3d912a
+                    break;
3d912a
+                }
3d912a
                 pubk->keyType = ecKey;
3d912a
                 pubk->u.ec.size = 0;
3d912a
 
3d912a
diff --git a/lib/freebl/dh.c b/lib/freebl/dh.c
3d912a
--- a/lib/freebl/dh.c
3d912a
+++ b/lib/freebl/dh.c
3d912a
@@ -210,7 +210,8 @@ DH_Derive(SECItem *publicValue,
3d912a
     unsigned int len = 0;
3d912a
     unsigned int nb;
3d912a
     unsigned char *secret = NULL;
3d912a
-    if (!publicValue || !prime || !privateValue || !derivedSecret) {
3d912a
+    if (!publicValue || !publicValue->len || !prime || !prime->len ||
3d912a
+        !privateValue || !privateValue->len || !derivedSecret) {
3d912a
         PORT_SetError(SEC_ERROR_INVALID_ARGS);
3d912a
         return SECFailure;
3d912a
     }
3d912a
diff --git a/lib/freebl/ec.c b/lib/freebl/ec.c
3d912a
--- a/lib/freebl/ec.c
3d912a
+++ b/lib/freebl/ec.c
3d912a
@@ -202,8 +202,8 @@ ec_NewKey(ECParams *ecParams, ECPrivateK
3d912a
 #endif
3d912a
     MP_DIGITS(&k) = 0;
3d912a
 
3d912a
-    if (!ecParams || !privKey || !privKeyBytes || (privKeyLen < 0) ||
3d912a
-        !ecParams->name) {
3d912a
+    if (!ecParams || ecParams->name == ECCurve_noName ||
3d912a
+        !privKey || !privKeyBytes || privKeyLen <= 0) {
3d912a
         PORT_SetError(SEC_ERROR_INVALID_ARGS);
3d912a
         return SECFailure;
3d912a
     }
3d912a
@@ -391,7 +391,7 @@ EC_NewKey(ECParams *ecParams, ECPrivateK
3d912a
     int len;
3d912a
     unsigned char *privKeyBytes = NULL;
3d912a
 
3d912a
-    if (!ecParams) {
3d912a
+    if (!ecParams || ecParams->name == ECCurve_noName || !privKey) {
3d912a
         PORT_SetError(SEC_ERROR_INVALID_ARGS);
3d912a
         return SECFailure;
3d912a
     }
3d912a
@@ -430,7 +430,8 @@ EC_ValidatePublicKey(ECParams *ecParams,
3d912a
     mp_err err = MP_OKAY;
3d912a
     int len;
3d912a
 
3d912a
-    if (!ecParams || !publicValue || !ecParams->name) {
3d912a
+    if (!ecParams || ecParams->name == ECCurve_noName ||
3d912a
+        !publicValue || !publicValue->len) {
3d912a
         PORT_SetError(SEC_ERROR_INVALID_ARGS);
3d912a
         return SECFailure;
3d912a
     }
3d912a
@@ -536,8 +537,9 @@ ECDH_Derive(SECItem *publicValue,
3d912a
     int i;
3d912a
 #endif
3d912a
 
3d912a
-    if (!publicValue || !ecParams || !privateValue || !derivedSecret ||
3d912a
-        !ecParams->name) {
3d912a
+    if (!publicValue || !publicValue->len ||
3d912a
+        !ecParams || ecParams->name == ECCurve_noName ||
3d912a
+        !privateValue || !privateValue->len || !derivedSecret) {
3d912a
         PORT_SetError(SEC_ERROR_INVALID_ARGS);
3d912a
         return SECFailure;
3d912a
     }
3d912a
diff --git a/lib/util/quickder.c b/lib/util/quickder.c
3d912a
--- a/lib/util/quickder.c
3d912a
+++ b/lib/util/quickder.c
3d912a
@@ -757,6 +757,13 @@ DecodeItem(void* dest,
3d912a
                         }
3d912a
 
3d912a
                         case SEC_ASN1_BIT_STRING: {
3d912a
+                            /* Can't be 8 or more spare bits, or any spare bits
3d912a
+			     * if there are no octets. */
3d912a
+                            if (temp.data[0] >= 8 || (temp.data[0] > 0 && temp.len == 1)) {
3d912a
+                                PORT_SetError(SEC_ERROR_BAD_DER);
3d912a
+                                rv = SECFailure;
3d912a
+                                break;
3d912a
+                            }
3d912a
                             /* change the length in the SECItem to be the number
3d912a
                                of bits */
3d912a
                             temp.len = (temp.len - 1) * 8 - (temp.data[0] & 0x7);
3d912a
# HG changeset patch
3d912a
# User Kevin Jacobs <kjacobs@mozilla.com>
3d912a
# Date 1561145635 0
3d912a
#      Fri Jun 21 19:33:55 2019 +0000
3d912a
# Branch NSS_3_44_BRANCH
3d912a
# Node ID 416a8f7cf8986103b4d74694aac1198edbb08b3e
3d912a
# Parent  fb9932d6e083322e7b5dfcd3d6e67477e0bb075a
3d912a
Bug 1515342 - Ignore spki decode failures on negative (expect_fail) tests. r=jcj
3d912a
3d912a
Differential Revision: https://phabricator.services.mozilla.com/D35565
3d912a
3d912a
diff --git a/gtests/pk11_gtest/pk11_curve25519_unittest.cc b/gtests/pk11_gtest/pk11_curve25519_unittest.cc
3d912a
--- a/gtests/pk11_gtest/pk11_curve25519_unittest.cc
3d912a
+++ b/gtests/pk11_gtest/pk11_curve25519_unittest.cc
3d912a
@@ -40,6 +40,9 @@ class Pkcs11Curve25519Test
3d912a
 
3d912a
     ScopedCERTSubjectPublicKeyInfo certSpki(
3d912a
         SECKEY_DecodeDERSubjectPublicKeyInfo(&spkiItem));
3d912a
+    if (!expect_success && !certSpki) {
3d912a
+      return;
3d912a
+    }
3d912a
     ASSERT_TRUE(certSpki);
3d912a
 
3d912a
     ScopedSECKEYPublicKey pubKey(SECKEY_ExtractPublicKey(certSpki.get()));