diff options
author | Brad Fitzpatrick <bradfitz@golang.org> | 2022-03-17 09:39:46 -0700 |
---|---|---|
committer | Cherry Mui <cherryyz@google.com> | 2022-04-04 21:16:23 +0000 |
commit | 30d9077a3485b1ff7cfd877fcc16ecb64dc3a4ae (patch) | |
tree | a4dd7e8569e478a7311b0b8b61ad0081e7aab826 | |
parent | 6412231192755c1d9f8a671614ace8b45fcbe49c (diff) | |
download | go-30d9077a3485b1ff7cfd877fcc16ecb64dc3a4ae.tar.gz go-30d9077a3485b1ff7cfd877fcc16ecb64dc3a4ae.zip |
[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>
-rw-r--r-- | src/crypto/x509/root_darwin.go | 7 | ||||
-rw-r--r-- | src/crypto/x509/verify_test.go | 34 |
2 files changed, 40 insertions, 1 deletions
diff --git a/src/crypto/x509/root_darwin.go b/src/crypto/x509/root_darwin.go index 1ef9c0f71e..ad365f577e 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 f4ea08bbf5..100a8ff0f9 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") + } + }) +} |