aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBrad Fitzpatrick <bradfitz@golang.org>2022-03-17 09:39:46 -0700
committerCherry Mui <cherryyz@google.com>2022-04-04 21:16:23 +0000
commit30d9077a3485b1ff7cfd877fcc16ecb64dc3a4ae (patch)
treea4dd7e8569e478a7311b0b8b61ad0081e7aab826
parent6412231192755c1d9f8a671614ace8b45fcbe49c (diff)
downloadgo-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.go7
-rw-r--r--src/crypto/x509/verify_test.go34
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")
+ }
+ })
+}