[release-branch.go1.18] crypto/x509: fix Certificate.Verify crash

(Primarily from Josh)

Updates #51759
Fixes #51763
Fixes CVE-2022-27536

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>
(cherry picked from commit 0fca8a8f25cf4636fd980e72ba0bded4230922de)
Reviewed-on: https://go-review.googlesource.com/c/go/+/394655
Trust: Roland Shoemaker <roland@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>

diff --git a/src/crypto/x509/root_darwin.go b/src/crypto/x509/root_darwin.go
index 1ef9c0f71e54506a1cfe90c752e4a9c2b852c367..ad365f577e79634644672a03a47294931776e5d7 100644 (file)
--- 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 f4ea08bbf5516a7ee53108ea5a945ebc0c3aff92..100a8ff0f94ae4c134d7ba61beacbd855fa4f81c 100644 (file)
--- 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")
+               }
+       })
+}