nss: reject DH generators and public values equal to zero or one.

BUG=124836
TEST=none

Review URL: http://codereview.chromium.org/10173004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@133948 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 84 insertions, 4 deletions

diff --git a/net/third_party/nss/README.chromium b/net/third_party/nss/README.chromium
index d7110f0..b5105a0 100644
--- a/net/third_party/nss/README.chromium
+++ b/net/third_party/nss/README.chromium
@@ -68,6 +68,9 @@ Patches:
   * Enable False Start only when the server supports NPN.
     patches/falsestartnpn.patch
 
+  * Reject DH generators and public values equal to zero or one.
+    patches/dhvalues.patch
+
 Apply the patches to NSS by running the patches/applypatches.sh script.  Read
 the comments at the top of patches/applypatches.sh for instructions.
 
diff --git a/net/third_party/nss/patches/applypatches.sh b/net/third_party/nss/patches/applypatches.sh
index debd48c..55167bf 100755
--- a/net/third_party/nss/patches/applypatches.sh
+++ b/net/third_party/nss/patches/applypatches.sh
@@ -37,3 +37,5 @@ patch -p6 < $patches_dir/encryptedclientcerts.patch
 patch -p4 < $patches_dir/dtls.patch
 
 patch -p5 < $patches_dir/falsestartnpn.patch
+
+patch -p5 < $patches_dir/dhvalues.patch
diff --git a/net/third_party/nss/patches/dhvalues.patch b/net/third_party/nss/patches/dhvalues.patch
new file mode 100644
index 0000000..5d8ef28
--- /dev/null
+++ b/net/third_party/nss/patches/dhvalues.patch
@@ -0,0 +1,53 @@
+diff --git a/net/third_party/nss/ssl/ssl3con.c b/net/third_party/nss/ssl/ssl3con.c
+index b6f4313..5476fa5 100644
+--- a/net/third_party/nss/ssl/ssl3con.c
++++ b/net/third_party/nss/ssl/ssl3con.c
+@@ -5505,6 +5505,30 @@ loser:
+     return SECFailure;
+ }
+ 
++/* ssl3_BigIntGreaterThan1 returns true iff |mpint|, taken as an unsigned,
++ * big-endian integer is > 1 */
++static PRBool
++ssl3_BigIntGreaterThan1(const SECItem* mpint) {
++    unsigned char firstNonZeroByte = 0;
++    unsigned int i;
++
++    for (i = 0; i < mpint->len; i++) {
++	if (mpint->data[i]) {
++	    firstNonZeroByte = mpint->data[i];
++	    break;
++	}
++    }
++
++    if (firstNonZeroByte == 0)
++	return PR_FALSE;
++    if (firstNonZeroByte > 1)
++	return PR_TRUE;
++
++    // firstNonZeroByte == 1, therefore mpint > 1 iff the first non-zero byte
++    // is followed by another byte.
++    return (i < mpint->len - 1);
++}
++
+ /* Called from ssl3_HandleHandshakeMessage() when it has deciphered a complete
+  * ssl3 ServerKeyExchange message.
+  * Caller must hold Handshake and RecvBuf locks.
+@@ -5636,15 +5660,13 @@ ssl3_HandleServerKeyExchange(sslSocket *ss, SSL3Opaque *b, PRUint32 length)
+     	if (rv != SECSuccess) {
+ 	    goto loser;		/* malformed. */
+ 	}
+-	if (dh_g.len == 0 || dh_g.len > dh_p.len + 1 ||
+-	   (dh_g.len == 1 && dh_g.data[0] == 0))
++	if (dh_g.len > dh_p.len || !ssl3_BigIntGreaterThan1(&dh_g))
+ 	    goto alert_loser;
+     	rv = ssl3_ConsumeHandshakeVariable(ss, &dh_Ys, 2, &b, &length);
+     	if (rv != SECSuccess) {
+ 	    goto loser;		/* malformed. */
+ 	}
+-	if (dh_Ys.len == 0 || dh_Ys.len > dh_p.len + 1 ||
+-	   (dh_Ys.len == 1 && dh_Ys.data[0] == 0))
++	if (dh_Ys.len > dh_p.len || !ssl3_BigIntGreaterThan1(&dh_Ys))
+ 	    goto alert_loser;
+     	rv = ssl3_ConsumeHandshakeVariable(ss, &signature, 2, &b, &length);
+     	if (rv != SECSuccess) {
diff --git a/net/third_party/nss/ssl/ssl3con.c b/net/third_party/nss/ssl/ssl3con.c
index b6f4313..5476fa5 100644
--- a/net/third_party/nss/ssl/ssl3con.c
+++ b/net/third_party/nss/ssl/ssl3con.c
@@ -5505,6 +5505,30 @@ loser:
     return SECFailure;
 }
 
+/* ssl3_BigIntGreaterThan1 returns true iff |mpint|, taken as an unsigned,
+ * big-endian integer is > 1 */
+static PRBool
+ssl3_BigIntGreaterThan1(const SECItem* mpint) {
+    unsigned char firstNonZeroByte = 0;
+    unsigned int i;
+
+    for (i = 0; i < mpint->len; i++) {
+	if (mpint->data[i]) {
+	    firstNonZeroByte = mpint->data[i];
+	    break;
+	}
+    }
+
+    if (firstNonZeroByte == 0)
+	return PR_FALSE;
+    if (firstNonZeroByte > 1)
+	return PR_TRUE;
+
+    // firstNonZeroByte == 1, therefore mpint > 1 iff the first non-zero byte
+    // is followed by another byte.
+    return (i < mpint->len - 1);
+}
+
 /* Called from ssl3_HandleHandshakeMessage() when it has deciphered a complete
  * ssl3 ServerKeyExchange message.
  * Caller must hold Handshake and RecvBuf locks.
@@ -5636,15 +5660,13 @@ ssl3_HandleServerKeyExchange(sslSocket *ss, SSL3Opaque *b, PRUint32 length)
     	if (rv != SECSuccess) {
 	    goto loser;		/* malformed. */
 	}
-	if (dh_g.len == 0 || dh_g.len > dh_p.len + 1 ||
-	   (dh_g.len == 1 && dh_g.data[0] == 0))
+	if (dh_g.len > dh_p.len || !ssl3_BigIntGreaterThan1(&dh_g))
 	    goto alert_loser;
     	rv = ssl3_ConsumeHandshakeVariable(ss, &dh_Ys, 2, &b, &length);
     	if (rv != SECSuccess) {
 	    goto loser;		/* malformed. */
 	}
-	if (dh_Ys.len == 0 || dh_Ys.len > dh_p.len + 1 ||
-	   (dh_Ys.len == 1 && dh_Ys.data[0] == 0))
+	if (dh_Ys.len > dh_p.len || !ssl3_BigIntGreaterThan1(&dh_Ys))
 	    goto alert_loser;
     	rv = ssl3_ConsumeHandshakeVariable(ss, &signature, 2, &b, &length);
     	if (rv != SECSuccess) {