]> Cypherpunks repositories - gostls13.git/commitdiff
[release-branch.go1.24] encoding/asn1: prevent memory exhaustion when parsing using...
authorNicholas Husin <husin@google.com>
Wed, 3 Sep 2025 13:30:56 +0000 (09:30 -0400)
committerGopher Robot <gobot@golang.org>
Tue, 7 Oct 2025 18:00:54 +0000 (11:00 -0700)
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 #75704
Fixes CVE-2025-58185

Change-Id: Id50e76187eda43f594be75e516b9ca1d2ae6f428
Reviewed-on: https://go-internal-review.googlesource.com/c/go/+/2700
Reviewed-by: Roland Shoemaker <bracewell@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-on: https://go-internal-review.googlesource.com/c/go/+/2984
Reviewed-by: Nicholas Husin <husin@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/709841
Reviewed-by: Carlos Amedee <carlos@golang.org>
Auto-Submit: Michael Pratt <mpratt@google.com>
TryBot-Bypass: Michael Pratt <mpratt@google.com>

src/encoding/asn1/asn1.go
src/encoding/asn1/asn1_test.go
src/go/build/deps_test.go

index 488fb9b1e096f10be13d8fae567247799b78f519..e1f4cba0d5889b4d633548ce39bdc14aee502b07 100644 (file)
@@ -22,6 +22,7 @@ package asn1
 import (
        "errors"
        "fmt"
+       "internal/saferio"
        "math"
        "math/big"
        "reflect"
@@ -635,10 +636,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
index 9a605e245c13698f324281ba3fdec9dffdba8184..249d4e44cb5826258e6d93a0f85c58b24d225cd5 100644 (file)
@@ -7,10 +7,12 @@ package asn1
 import (
        "bytes"
        "encoding/hex"
+       "errors"
        "fmt"
        "math"
        "math/big"
        "reflect"
+       "runtime"
        "strings"
        "testing"
        "time"
@@ -1175,3 +1177,39 @@ func BenchmarkObjectIdentifierString(b *testing.B) {
                _ = oidPublicKeyRSA.String()
        }
 }
+
+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)
+       }
+}
index 5694e435f65ccbf9381cbb7f3f898b0d681507f8..48d8e4be177362571e9bd0991eb1e10cb916ac87 100644 (file)
@@ -535,7 +535,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/rand
        < crypto/ed25519 # depends on crypto/rand.Reader