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>

diff --git a/src/crypto/dsa/dsa.go b/src/crypto/dsa/dsa.go
index bc8e2f99bdc75a33cd04ba8c89eb357a3cd10147..43826bcb559ebba0a0689a9b26c85f7e0e79b4b9 100644 (file)
--- 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 7fc246bc2bdf84306743fb8bab85e942b9c08893..7332a3a54036a0116d5d42766985181e173b26e2 100644 (file)
--- 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.