From e0f655bf3f96410f90756f49532bc6a1851855ca Mon Sep 17 00:00:00 2001 From: Nicholas Husin Date: Wed, 3 Sep 2025 09:30:56 -0400 Subject: [PATCH] [release-branch.go1.25] encoding/asn1: prevent memory exhaustion when parsing using internal/saferio Within parseSequenceOf, reflect.MakeSlice is being used to pre-allocate a slice that is needed in order to fully validate the given DER payload. The size of the slice allocated are also multiple times larger than the input DER: - When using asn1.Unmarshal directly, the allocated slice is ~28x larger. - When passing in DER using x509.ParseCertificateRequest, the allocated slice is ~48x larger. - When passing in DER using ocsp.ParseResponse, the allocated slice is ~137x larger. As a result, a malicious actor can craft a big empty DER payload, resulting in an unnecessary large allocation of memories. This can be a way to cause memory exhaustion. To prevent this, we now use SliceCapWithSize within internal/saferio to enforce a memory allocation cap. Thanks to Jakub Ciolek for reporting this issue. For #75671 Fixes #75705 Fixes CVE-2025-58185 Change-Id: Id50e76187eda43f594be75e516b9ca1d2ae6f428 Reviewed-on: https://go-internal-review.googlesource.com/c/go/+/2700 Reviewed-by: Roland Shoemaker Reviewed-by: Damien Neil Reviewed-on: https://go-internal-review.googlesource.com/c/go/+/2966 Reviewed-by: Nicholas Husin Commit-Queue: Roland Shoemaker Reviewed-on: https://go-review.googlesource.com/c/go/+/709850 TryBot-Bypass: Michael Pratt Reviewed-by: Carlos Amedee Auto-Submit: Michael Pratt --- src/encoding/asn1/asn1.go | 10 ++++++++- src/encoding/asn1/asn1_test.go | 38 ++++++++++++++++++++++++++++++++++ src/go/build/deps_test.go | 2 +- 3 files changed, 48 insertions(+), 2 deletions(-) diff --git a/src/encoding/asn1/asn1.go b/src/encoding/asn1/asn1.go index 0b64f06d36..f4be515b98 100644 --- a/src/encoding/asn1/asn1.go +++ b/src/encoding/asn1/asn1.go @@ -22,6 +22,7 @@ package asn1 import ( "errors" "fmt" + "internal/saferio" "math" "math/big" "reflect" @@ -666,10 +667,17 @@ func parseSequenceOf(bytes []byte, sliceType reflect.Type, elemType reflect.Type offset += t.length numElements++ } - ret = reflect.MakeSlice(sliceType, numElements, numElements) + elemSize := uint64(elemType.Size()) + safeCap := saferio.SliceCapWithSize(elemSize, uint64(numElements)) + if safeCap < 0 { + err = SyntaxError{fmt.Sprintf("%s slice too big: %d elements of %d bytes", elemType.Kind(), numElements, elemSize)} + return + } + ret = reflect.MakeSlice(sliceType, 0, safeCap) params := fieldParameters{} offset := 0 for i := 0; i < numElements; i++ { + ret = reflect.Append(ret, reflect.Zero(elemType)) offset, err = parseField(ret.Index(i), bytes, offset, params) if err != nil { return diff --git a/src/encoding/asn1/asn1_test.go b/src/encoding/asn1/asn1_test.go index 0597740bd5..41cc0ba50e 100644 --- a/src/encoding/asn1/asn1_test.go +++ b/src/encoding/asn1/asn1_test.go @@ -7,10 +7,12 @@ package asn1 import ( "bytes" "encoding/hex" + "errors" "fmt" "math" "math/big" "reflect" + "runtime" "strings" "testing" "time" @@ -1216,3 +1218,39 @@ func TestImplicitTypeRoundtrip(t *testing.T) { t.Fatalf("Unexpected diff after roundtripping struct\na: %#v\nb: %#v", a, b) } } + +func TestParsingMemoryConsumption(t *testing.T) { + // Craft a syntatically valid, but empty, ~10 MB DER bomb. A successful + // unmarshal of this bomb should yield ~280 MB. However, the parsing should + // fail due to the empty content; and, in such cases, we want to make sure + // that we do not unnecessarily allocate memories. + derBomb := make([]byte, 10_000_000) + for i := range derBomb { + derBomb[i] = 0x30 + } + derBomb = append([]byte{0x30, 0x83, 0x98, 0x96, 0x80}, derBomb...) + + var m runtime.MemStats + runtime.GC() + runtime.ReadMemStats(&m) + memBefore := m.TotalAlloc + + var out []struct { + Id []int + Critical bool `asn1:"optional"` + Value []byte + } + _, err := Unmarshal(derBomb, &out) + if !errors.As(err, &SyntaxError{}) { + t.Fatalf("Incorrect error result: want (%v), but got (%v) instead", &SyntaxError{}, err) + } + + runtime.ReadMemStats(&m) + memDiff := m.TotalAlloc - memBefore + + // Ensure that the memory allocated does not exceed 10<<21 (~20 MB) when + // the parsing fails. + if memDiff > 10<<21 { + t.Errorf("Too much memory allocated while parsing DER: %v MiB", memDiff/1024/1024) + } +} diff --git a/src/go/build/deps_test.go b/src/go/build/deps_test.go index b3ca2a017e..641d1a325a 100644 --- a/src/go/build/deps_test.go +++ b/src/go/build/deps_test.go @@ -559,7 +559,7 @@ var depsRules = ` # CRYPTO-MATH is crypto that exposes math/big APIs - no cgo, net; fmt now ok. - CRYPTO, FMT, math/big + CRYPTO, FMT, math/big, internal/saferio < crypto/internal/boring/bbig < crypto/internal/fips140cache < crypto/rand -- 2.52.0