From 0fca8a8f25cf4636fd980e72ba0bded4230922de Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Thu, 17 Mar 2022 09:39:46 -0700 Subject: [PATCH] crypto/x509: fix Certificate.Verify crash (Primarily from Josh) Fixes #51759 Co-authored-by: Josh Bleecher Snyder Change-Id: I0a6f2623b57750abd13d5e194b5c6ffa3be6bf72 Reviewed-on: https://go-review.googlesource.com/c/go/+/393655 Trust: Brad Fitzpatrick Run-TryBot: Brad Fitzpatrick Reviewed-by: Roland Shoemaker TryBot-Result: Gopher Robot --- src/crypto/x509/root_darwin.go | 7 ++++++- src/crypto/x509/verify_test.go | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/src/crypto/x509/root_darwin.go b/src/crypto/x509/root_darwin.go index 1ef9c0f71e5..ad365f577e7 100644 --- 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 index f4ea08bbf55..100a8ff0f94 100644 --- 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") + } + }) +}