From b13ce14c4a6aa59b7b041ad2b6eed2d23e15b574 Mon Sep 17 00:00:00 2001 From: Dmitri Shuralyov Date: Tue, 28 Jan 2020 13:20:57 -0500 Subject: [PATCH] src/go.mod: import x/crypto/cryptobyte security fix for 32-bit archs cryptobyte: fix panic due to malformed ASN.1 inputs on 32-bit archs When int is 32 bits wide (on 32-bit architectures like 386 and arm), an overflow could occur, causing a panic, due to malformed ASN.1 being passed to any of the ASN1 methods of String. Tested on linux/386 and darwin/amd64. This fixes CVE-2020-7919 and was found thanks to the Project Wycheproof test vectors. Change-Id: I8c9696a8bfad1b40ec877cd740dba3467d66ab54 Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/645211 Reviewed-by: Katie Hockman Reviewed-by: Adam Langley Reviewed-on: https://go-review.googlesource.com/c/crypto/+/216677 Run-TryBot: Katie Hockman Reviewed-by: Dmitri Shuralyov Reviewed-by: Filippo Valsorda TryBot-Result: Gobot Gobot x/crypto/cryptobyte is used in crypto/x509 for parsing certificates. Malformed certificates might cause a panic during parsing on 32-bit architectures (like arm and 386). Change-Id: I840feb54eba880dbb96780ef7adcade073c4c4e3 Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/647741 Reviewed-by: Katie Hockman Reviewed-on: https://go-review.googlesource.com/c/go/+/216680 Reviewed-by: Katie Hockman --- src/go.mod | 2 +- src/go.sum | 4 ++-- src/vendor/golang.org/x/crypto/cryptobyte/asn1.go | 5 +++-- src/vendor/golang.org/x/crypto/cryptobyte/string.go | 7 +------ src/vendor/modules.txt | 2 +- 5 files changed, 8 insertions(+), 12 deletions(-) diff --git a/src/go.mod b/src/go.mod index 3ef0710745d..72114080ceb 100644 --- a/src/go.mod +++ b/src/go.mod @@ -3,7 +3,7 @@ module std go 1.14 require ( - golang.org/x/crypto v0.0.0-20200109152110-61a87790db17 + golang.org/x/crypto v0.0.0-20200128174031-69ecbb4d6d5d golang.org/x/net v0.0.0-20191126235420-ef20fe5d7933 golang.org/x/sys v0.0.0-20190529130038-5219a1e1c5f8 // indirect golang.org/x/text v0.3.3-0.20191031172631-4b67af870c6f // indirect diff --git a/src/go.sum b/src/go.sum index 697caf4ece9..9f24502dc2a 100644 --- a/src/go.sum +++ b/src/go.sum @@ -1,6 +1,6 @@ golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= -golang.org/x/crypto v0.0.0-20200109152110-61a87790db17 h1:nVJ3guKA9qdkEQ3TUdXI9QSINo2CUPM/cySEvw2w8I0= -golang.org/x/crypto v0.0.0-20200109152110-61a87790db17/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= +golang.org/x/crypto v0.0.0-20200128174031-69ecbb4d6d5d h1:9FCpayM9Egr1baVnV1SX0H87m+XB0B8S0hAMi99X/3U= +golang.org/x/crypto v0.0.0-20200128174031-69ecbb4d6d5d/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20191126235420-ef20fe5d7933 h1:e6HwijUxhDe+hPNjZQQn9bA5PW3vNmnN64U2ZW759Lk= golang.org/x/net v0.0.0-20191126235420-ef20fe5d7933/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= diff --git a/src/vendor/golang.org/x/crypto/cryptobyte/asn1.go b/src/vendor/golang.org/x/crypto/cryptobyte/asn1.go index 528b9bff671..f930f7e5266 100644 --- a/src/vendor/golang.org/x/crypto/cryptobyte/asn1.go +++ b/src/vendor/golang.org/x/crypto/cryptobyte/asn1.go @@ -470,7 +470,8 @@ func (s *String) ReadASN1GeneralizedTime(out *time.Time) bool { // It reports whether the read was successful. func (s *String) ReadASN1BitString(out *encoding_asn1.BitString) bool { var bytes String - if !s.ReadASN1(&bytes, asn1.BIT_STRING) || len(bytes) == 0 { + if !s.ReadASN1(&bytes, asn1.BIT_STRING) || len(bytes) == 0 || + len(bytes)*8/8 != len(bytes) { return false } @@ -740,7 +741,7 @@ func (s *String) readASN1(out *String, outTag *asn1.Tag, skipHeader bool) bool { length = headerLen + len32 } - if uint32(int(length)) != length || !s.ReadBytes((*[]byte)(out), int(length)) { + if int(length) < 0 || !s.ReadBytes((*[]byte)(out), int(length)) { return false } if skipHeader && !out.Skip(int(headerLen)) { diff --git a/src/vendor/golang.org/x/crypto/cryptobyte/string.go b/src/vendor/golang.org/x/crypto/cryptobyte/string.go index 39bf98aeead..589d297e6be 100644 --- a/src/vendor/golang.org/x/crypto/cryptobyte/string.go +++ b/src/vendor/golang.org/x/crypto/cryptobyte/string.go @@ -24,7 +24,7 @@ type String []byte // read advances a String by n bytes and returns them. If less than n bytes // remain, it returns nil. func (s *String) read(n int) []byte { - if len(*s) < n { + if len(*s) < n || n < 0 { return nil } v := (*s)[:n] @@ -105,11 +105,6 @@ func (s *String) readLengthPrefixed(lenLen int, outChild *String) bool { length = length << 8 length = length | uint32(b) } - if int(length) < 0 { - // This currently cannot overflow because we read uint24 at most, but check - // anyway in case that changes in the future. - return false - } v := s.read(int(length)) if v == nil { return false diff --git a/src/vendor/modules.txt b/src/vendor/modules.txt index ba6bd9845c0..0944c9a5330 100644 --- a/src/vendor/modules.txt +++ b/src/vendor/modules.txt @@ -1,4 +1,4 @@ -# golang.org/x/crypto v0.0.0-20200109152110-61a87790db17 +# golang.org/x/crypto v0.0.0-20200128174031-69ecbb4d6d5d ## explicit golang.org/x/crypto/chacha20 golang.org/x/crypto/chacha20poly1305