From 552987fdbf4c2bc9641016fd323c3ae5d3a0d9a3 Mon Sep 17 00:00:00 2001
From: Katie Hockman <katie@golang.org>
Date: Mon, 14 Oct 2019 16:42:21 -0400
Subject: [PATCH] crypto/dsa: prevent bad public keys from causing panic

dsa.Verify might currently use a nil s inverse in a
multiplication if the public key contains a non-prime Q,
causing a panic. Change this to check that the mod
inverse exists before using it.

Fixes CVE-2019-17596

Fixes #34960

Change-Id: I94d5f3cc38f1b5d52d38dcb1d253c71b7fd1cae7
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/572809
Reviewed-by: Filippo Valsorda <valsorda@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/205441
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
---
 src/crypto/dsa/dsa.go      |  3 +++
 src/crypto/dsa/dsa_test.go | 15 +++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/src/crypto/dsa/dsa.go b/src/crypto/dsa/dsa.go
index bc8e2f99bdc75..43826bcb559eb 100644
--- a/src/crypto/dsa/dsa.go
+++ b/src/crypto/dsa/dsa.go
@@ -279,6 +279,9 @@ func Verify(pub *PublicKey, hash []byte, r, s *big.Int) bool {
 	}
 
 	w := new(big.Int).ModInverse(s, pub.Q)
+	if w == nil {
+		return false
+	}
 
 	n := pub.Q.BitLen()
 	if n%8 != 0 {
diff --git a/src/crypto/dsa/dsa_test.go b/src/crypto/dsa/dsa_test.go
index 7fc246bc2bdf8..7332a3a54036a 100644
--- a/src/crypto/dsa/dsa_test.go
+++ b/src/crypto/dsa/dsa_test.go
@@ -97,6 +97,21 @@ func TestSignAndVerify(t *testing.T) {
 	testSignAndVerify(t, 0, &priv)
 }
 
+func TestSignAndVerifyWithBadPublicKey(t *testing.T) {
+	pub := PublicKey{
+		Parameters: Parameters{
+			P: fromHex("A9B5B793FB4785793D246BAE77E8FF63CA52F442DA763C440259919FE1BC1D6065A9350637A04F75A2F039401D49F08E066C4D275A5A65DA5684BC563C14289D7AB8A67163BFBF79D85972619AD2CFF55AB0EE77A9002B0EF96293BDD0F42685EBB2C66C327079F6C98000FBCB79AACDE1BC6F9D5C7B1A97E3D9D54ED7951FEF"),
+			Q: fromHex("FA"),
+			G: fromHex("634364FC25248933D01D1993ECABD0657CC0CB2CEED7ED2E3E8AECDFCDC4A25C3B15E9E3B163ACA2984B5539181F3EFF1A5E8903D71D5B95DA4F27202B77D2C44B430BB53741A8D59A8F86887525C9F2A6A5980A195EAA7F2FF910064301DEF89D3AA213E1FAC7768D89365318E370AF54A112EFBA9246D9158386BA1B4EEFDA"),
+		},
+		Y: fromHex("32969E5780CFE1C849A1C276D7AEB4F38A23B591739AA2FE197349AEEBD31366AEE5EB7E6C6DDB7C57D02432B30DB5AA66D9884299FAA72568944E4EEDC92EA3FBC6F39F53412FBCC563208F7C15B737AC8910DBC2D9C9B8C001E72FDC40EB694AB1F06A5A2DBD18D9E36C66F31F566742F11EC0A52E9F7B89355C02FB5D32D2"),
+	}
+
+	if Verify(&pub, []byte("testing"), fromHex("2"), fromHex("4")) {
+		t.Errorf("Verify unexpected success with non-existant mod inverse of Q")
+	}
+}
+
 func TestSigningWithDegenerateKeys(t *testing.T) {
 	// Signing with degenerate private keys should not cause an infinite
 	// loop.