From 83e9a97f62af41e65e37e096a4ed71f6a59d183e Mon Sep 17 00:00:00 2001 From: Filippo Valsorda Date: Tue, 22 Mar 2022 13:35:11 -0400 Subject: [PATCH] crypto/x509/internal/macos: return errors when CFRef might be NULL Updates #51759 Change-Id: Ib73fa5ec62d90c7e595150217b048158789f1afd Reviewed-on: https://go-review.googlesource.com/c/go/+/394674 Run-TryBot: Filippo Valsorda Trust: Josh Bleecher Snyder TryBot-Result: Gopher Robot Reviewed-by: Roland Shoemaker --- src/crypto/x509/internal/macos/corefoundation.go | 14 ++++++++++---- src/crypto/x509/internal/macos/security.go | 11 ++++++++--- src/crypto/x509/root_darwin.go | 8 ++++---- 3 files changed, 22 insertions(+), 11 deletions(-) diff --git a/src/crypto/x509/internal/macos/corefoundation.go b/src/crypto/x509/internal/macos/corefoundation.go index 75c212910b..eb91a5db6e 100644 --- a/src/crypto/x509/internal/macos/corefoundation.go +++ b/src/crypto/x509/internal/macos/corefoundation.go @@ -37,9 +37,12 @@ func CFDataToSlice(data CFRef) []byte { } // CFStringToString returns a Go string representation of the passed -// in CFString. +// in CFString, or an empty string if it's invalid. func CFStringToString(ref CFRef) string { - data := CFStringCreateExternalRepresentation(ref) + data, err := CFStringCreateExternalRepresentation(ref) + if err != nil { + return "" + } b := CFDataToSlice(data) CFRelease(data) return string(b) @@ -186,9 +189,12 @@ func x509_CFErrorCopyDescription_trampoline() //go:cgo_import_dynamic x509_CFStringCreateExternalRepresentation CFStringCreateExternalRepresentation "/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation" -func CFStringCreateExternalRepresentation(strRef CFRef) CFRef { +func CFStringCreateExternalRepresentation(strRef CFRef) (CFRef, error) { ret := syscall(abi.FuncPCABI0(x509_CFStringCreateExternalRepresentation_trampoline), kCFAllocatorDefault, uintptr(strRef), kCFStringEncodingUTF8, 0, 0, 0) - return CFRef(ret) + if ret == 0 { + return 0, errors.New("string can't be represented as UTF-8") + } + return CFRef(ret), nil } func x509_CFStringCreateExternalRepresentation_trampoline() diff --git a/src/crypto/x509/internal/macos/security.go b/src/crypto/x509/internal/macos/security.go index ef64bda49f..381d918a94 100644 --- a/src/crypto/x509/internal/macos/security.go +++ b/src/crypto/x509/internal/macos/security.go @@ -131,11 +131,16 @@ func x509_SecTrustCreateWithCertificates_trampoline() //go:cgo_import_dynamic x509_SecCertificateCreateWithData SecCertificateCreateWithData "/System/Library/Frameworks/Security.framework/Versions/A/Security" -func SecCertificateCreateWithData(b []byte) CFRef { +func SecCertificateCreateWithData(b []byte) (CFRef, error) { data := BytesToCFData(b) + defer CFRelease(data) ret := syscall(abi.FuncPCABI0(x509_SecCertificateCreateWithData_trampoline), kCFAllocatorDefault, uintptr(data), 0, 0, 0, 0) - CFRelease(data) - return CFRef(ret) + // Returns NULL if the data passed in the data parameter is not a valid + // DER-encoded X.509 certificate. + if ret == 0 { + return 0, errors.New("SecCertificateCreateWithData: invalid certificate") + } + return CFRef(ret), nil } func x509_SecCertificateCreateWithData_trampoline() diff --git a/src/crypto/x509/root_darwin.go b/src/crypto/x509/root_darwin.go index ad365f577e..4759462653 100644 --- a/src/crypto/x509/root_darwin.go +++ b/src/crypto/x509/root_darwin.go @@ -12,8 +12,8 @@ import ( func (c *Certificate) systemVerify(opts *VerifyOptions) (chains [][]*Certificate, err error) { certs := macOS.CFArrayCreateMutable() defer macOS.ReleaseCFArray(certs) - leaf := macOS.SecCertificateCreateWithData(c.Raw) - if leaf == 0 { + leaf, err := macOS.SecCertificateCreateWithData(c.Raw) + if err != nil { return nil, errors.New("invalid leaf certificate") } macOS.CFArrayAppendValue(certs, leaf) @@ -23,8 +23,8 @@ func (c *Certificate) systemVerify(opts *VerifyOptions) (chains [][]*Certificate if err != nil { return nil, err } - sc := macOS.SecCertificateCreateWithData(c.Raw) - if sc != 0 { + sc, err := macOS.SecCertificateCreateWithData(c.Raw) + if err == nil { macOS.CFArrayAppendValue(certs, sc) } } -- 2.50.0