crypto/x509: fix Certificate.Verify crash

(Primarily from Josh) Fixes #51759 Co-authored-by: Josh Bleecher Snyder <josharian@gmail.com> Change-Id: I0a6f2623b57750abd13d5e194b5c6ffa3be6bf72 Reviewed-on: https://go-review.googlesource.com/c/go/+/393655 Trust: Brad Fitzpatrick <bradfitz@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Roland Shoemaker <roland@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org>
golang · Mar 17, 2022 · 0fca8a8 · 0fca8a8
1 parent c379c3d
commit 0fca8a8
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 1 deletion.
diff --git a/src/crypto/x509/root_darwin.go b/src/crypto/x509/root_darwin.go
@@ -13,6 +13,9 @@ func (c *Certificate) systemVerify(opts *VerifyOptions) (chains [][]*Certificate
 	certs := macOS.CFArrayCreateMutable()
 	defer macOS.ReleaseCFArray(certs)
 	leaf := macOS.SecCertificateCreateWithData(c.Raw)
+	if leaf == 0 {
+		return nil, errors.New("invalid leaf certificate")
+	}
 	macOS.CFArrayAppendValue(certs, leaf)
 	if opts.Intermediates != nil {
 		for _, lc := range opts.Intermediates.lazyCerts {
@@ -21,7 +24,9 @@ func (c *Certificate) systemVerify(opts *VerifyOptions) (chains [][]*Certificate
 				return nil, err
 			}
 			sc := macOS.SecCertificateCreateWithData(c.Raw)
-			macOS.CFArrayAppendValue(certs, sc)
+			if sc != 0 {
+				macOS.CFArrayAppendValue(certs, sc)
+			}
 		}
 	}
 

diff --git a/src/crypto/x509/verify_test.go b/src/crypto/x509/verify_test.go
@@ -1876,3 +1876,37 @@ func TestSystemRootsErrorUnwrap(t *testing.T) {
 		t.Error("errors.Is failed, wanted success")
 	}
 }
+
+func TestIssue51759(t *testing.T) {
+	// badCertData contains a cert that we parse as valid
+	// but that macOS SecCertificateCreateWithData rejects.
+	const badCertData = "0\x82\x01U0\x82\x01\a\xa0\x03\x02\x01\x02\x02\x01\x020\x05\x06\x03+ep0R1P0N\x06\x03U\x04\x03\x13Gderpkey8dc58100b2493614ee1692831a461f3f4dd3f9b3b088e244f887f81b4906ac260\x1e\x17\r220112235755Z\x17\r220313235755Z0R1P0N\x06\x03U\x04\x03\x13Gderpkey8dc58100b2493614ee1692831a461f3f4dd3f9b3b088e244f887f81b4906ac260*0\x05\x06\x03+ep\x03!\x00bA\xd8e\xadW\xcb\xefZ\x89\xb5\"\x1eR\x9d\xba\x0e:\x1042Q@\u007f\xbd\xfb{ks\x04\xd1£\x020\x000\x05\x06\x03+ep\x03A\x00[\xa7\x06y\x86(\x94\x97\x9eLwA\x00\x01x\xaa\xbc\xbd Ê]\n(΅!ف0\xf5\x9a%I\x19<\xffo\xf1\xeaaf@\xb1\xa7\xaf\xfd\xe9R\xc7\x0f\x8d&\xd5\xfc\x0f;Ϙ\x82\x84a\xbc\r"
+	badCert, err := ParseCertificate([]byte(badCertData))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	t.Run("leaf", func(t *testing.T) {
+		opts := VerifyOptions{}
+		_, err = badCert.Verify(opts)
+		if err == nil {
+			t.Fatal("expected error")
+		}
+	})
+
+	goodCert, err := certificateFromPEM(googleLeaf)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	t.Run("intermediate", func(t *testing.T) {
+		opts := VerifyOptions{
+			Intermediates: NewCertPool(),
+		}
+		opts.Intermediates.AddCert(badCert)
+		_, err = goodCert.Verify(opts)
+		if err == nil {
+			t.Fatal("expected error")
+		}
+	})
+}