Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,26 @@ on:
branches: [master]
pull_request:

permissions:
contents: read

jobs:
build:
name: Build
strategy:
matrix:
go: ["1.20", "1.21"]
go: ["1.25", "1.26"]
runs-on: ubuntu-latest
container: golang:${{ matrix.go }}-bookworm
steps:
- run: go install golang.org/x/tools/cmd/goimports@latest
- run: go install golang.org/x/lint/golint@latest
- uses: actions/checkout@v1
- uses: actions/checkout@v4
- run: ./gotest.sh

complete:
if: always()
needs: [build]
runs-on: ubuntu-latest
steps:
- if: contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled')
run: exit 1
Comment thread Fixed
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
module github.com/stellar/go-xdr

go 1.12
go 1.25
Comment thread
tamirms marked this conversation as resolved.
6 changes: 2 additions & 4 deletions gotest.sh
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
#!/bin/bash
# The script does automatic checking on a Go package and its sub-packages, including:
# 1. goimports (https://golang.org/x/tools/cmd/goimports)
# 2. golint (https://github.com/golang/lint)
# 3. go vet (https://golang.org/cmd/vet)
# 4. test coverage (https://blog.golang.org/cover)
# 2. go vet (https://golang.org/cmd/vet)
# 3. test coverage (https://blog.golang.org/cover)

set -ex

test -z "$(goimports -l -w .)"
test -z "$(golint ./... )"
go vet ./...
go test -covermode=atomic -race ./...
143 changes: 124 additions & 19 deletions xdr3/decode.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,14 @@
package xdr

import (
"errors"
"fmt"
"io"
"math"
"reflect"
"strconv"
"time"
"unsafe"
)

const maxInt32 = math.MaxInt32
Expand All @@ -31,7 +33,12 @@ var errMaxSlice = "data exceeds max slice limit"
var errIODecode = "%s while decoding %d bytes"

// DecodeDefaultMaxDepth is the default maximum decoding depth
const DecodeDefaultMaxDepth = 200
const DecodeDefaultMaxDepth = 250

// MaxPrealloc is the maximum number of elements pre-allocated when decoding
// variable-length arrays. Arrays larger than this are grown incrementally via
// append to keep memory proportional to data actually decoded.
const MaxPrealloc = 256

// DecodeOptions configures how Decoding is done.
type DecodeOptions struct {
Expand All @@ -47,6 +54,17 @@ type DecodeOptions struct {
// the provided io.Reader implements Len() (e.g. strings.Reader, bytes.Reader and bytes.Buffer do).
// Otherwise, no sanity checks will be done.
MaxInputLen int

// MaxMemoryBytes is an approximate limit on the cumulative in-memory size
// (in bytes) of Go objects created during a single decode operation. The
// decoder tracks unsafe.Sizeof for each decoded value (array elements,
// union arms, optional fields, opaque data) and aborts if the running
// total exceeds this limit. This is a best-effort bound: internal copies
// and over-allocation by the Go runtime may cause actual heap usage to be
// somewhat higher.
//
// If set to 0, no size limit is enforced (default).
MaxMemoryBytes int64
}

// DefaultDecodeOptions are the default decoding options.
Expand Down Expand Up @@ -125,10 +143,12 @@ type lenLeft interface {
// won't work.
type Decoder struct {
// used to minimize heap allocations during decoding
scratchBuf [8]byte
r io.Reader
l lenLeft
maxDepth uint
scratchBuf [8]byte
r io.Reader
l lenLeft
maxDepth uint
maxMemoryBytes int64
memoryBytes int64
}

// readerLenWrapper wraps a reader an initial length and provides a Len() method indicating
Expand Down Expand Up @@ -164,17 +184,18 @@ func NewDecoderWithOptions(r io.Reader, options DecodeOptions) *Decoder {
if maxDepth < 1 {
maxDepth = DecodeDefaultMaxDepth
}
mob := options.MaxMemoryBytes
if l, ok := r.(lenLeft); ok {
return &Decoder{r: r, l: l, maxDepth: maxDepth}
return &Decoder{r: r, l: l, maxDepth: maxDepth, maxMemoryBytes: mob}
}
if options.MaxInputLen > 0 {
rlw := &readerLenWrapper{
inner: r,
initialLen: options.MaxInputLen,
}
return &Decoder{r: rlw, l: rlw, maxDepth: maxDepth}
return &Decoder{r: rlw, l: rlw, maxDepth: maxDepth, maxMemoryBytes: mob}
}
return &Decoder{r: r, l: nil, maxDepth: options.MaxDepth}
return &Decoder{r: r, l: nil, maxDepth: maxDepth, maxMemoryBytes: mob}
}

// DecodeInt treats the next 4 bytes as an XDR encoded integer and returns the
Expand Down Expand Up @@ -387,6 +408,14 @@ func (d *Decoder) DecodeDouble() (float64, int, error) {
// RFC Section 4.9 - Fixed-Length Opaque Data
// Fixed-length uninterpreted data zero-padded to a multiple of four
func (d *Decoder) DecodeFixedOpaque(size int32) ([]byte, int, error) {
if size < 0 {
err := unmarshalError("DecodeFixedOpaque", ErrBadArguments,
"negative size", size, nil)
return nil, 0, err
}
if err := d.TrackOutputBytes(int64(size)); err != nil {
return nil, 0, err
}
out := make([]byte, size)
n, err := d.DecodeFixedOpaqueInplace(out)
if err != nil {
Expand Down Expand Up @@ -588,15 +617,11 @@ func (d *Decoder) decodeArray(v reflect.Value, ignoreOpaque bool, maxSize int, m
return n, err
}

// Allocate storage for the slice elements (the underlying array) if
// existing slice does not have enough capacity.
sliceLen := int(dataLen)
if v.Cap() < sliceLen {
v.Set(reflect.MakeSlice(v.Type(), sliceLen, sliceLen))
}
v.SetLen(sliceLen)

// Treat []byte (byte is alias for uint8) as opaque data unless ignored.
// DecodeFixedOpaque handles both tracking and allocation, so we skip
// pre-allocation here — SetBytes replaces the backing array directly.
if !ignoreOpaque && v.Type().Elem().Kind() == reflect.Uint8 {
data, n2, err := d.DecodeFixedOpaque(int32(sliceLen))
n += n2
Expand All @@ -607,13 +632,43 @@ func (d *Decoder) decodeArray(v reflect.Value, ignoreOpaque bool, maxSize int, m
return n, nil
}

// Decode each slice element.
for i := 0; i < sliceLen; i++ {
n2, err := d.decode(v.Index(i), 0, maxDepth)
n += n2
if err != nil {
// Cap pre-allocation to avoid memory amplification from untrusted inputs.
// The array length check above compares element count against remaining
// input bytes, but each element may be much larger in memory than on the
// wire. For large arrays, capping initial allocation and growing via
// append ensures memory usage is proportional to data actually decoded.
elemSize := int64(v.Type().Elem().Size())
if sliceLen <= MaxPrealloc {
Comment thread
tamirms marked this conversation as resolved.
// Small arrays: track total upfront, then pre-allocate and decode.
if err := d.TrackOutputBytes(elemSize * int64(sliceLen)); err != nil {
return n, err
}
if v.Cap() < sliceLen {
v.Set(reflect.MakeSlice(v.Type(), sliceLen, sliceLen))
}
v.SetLen(sliceLen)
for i := 0; i < sliceLen; i++ {
n2, err := d.decode(v.Index(i), 0, maxDepth)
n += n2
if err != nil {
return n, err
}
}
} else {
// Large arrays: cap initial allocation, track and grow per element.
v.Set(reflect.MakeSlice(v.Type(), 0, MaxPrealloc))
zeroElem := reflect.Zero(v.Type().Elem())
for i := 0; i < sliceLen; i++ {
if err := d.TrackOutputBytes(elemSize); err != nil {
return n, err
}
v.Set(reflect.Append(v, zeroElem))
Comment thread
tamirms marked this conversation as resolved.
n2, err := d.decode(v.Index(i), 0, maxDepth)
n += n2
if err != nil {
return n, err
}
}
}
return n, nil
}
Expand Down Expand Up @@ -675,6 +730,10 @@ func (d *Decoder) decodeUnion(v reflect.Value, maxDepth uint) (int, error) {
vv := v.FieldByName(arm)

vvet := vv.Type().Elem()
// Track the heap allocation for the pointer-typed union arm before allocating.
if err := d.TrackOutputBytes(int64(vvet.Size())); err != nil {
return n, err
}
vv.Set(reflect.New(vvet))

field, ok := v.Type().FieldByName(arm)
Expand Down Expand Up @@ -823,14 +882,22 @@ func (d *Decoder) decodeMap(v reflect.Value, maxDepth uint) (int, error) {
// Decode each key and value according to their type.
keyType := vt.Key()
elemType := vt.Elem()
keySize := int64(keyType.Size())
elemSize := int64(elemType.Size())
for i := uint32(0); i < dataLen; i++ {
if err := d.TrackOutputBytes(keySize); err != nil {
return n, err
}
key := reflect.New(keyType).Elem()
n2, err := d.decode(key, 0, maxDepth)
n += n2
if err != nil {
return n, err
}

if err := d.TrackOutputBytes(elemSize); err != nil {
return n, err
}
val := reflect.New(elemType).Elem()
n2, err = d.decode(val, 0, maxDepth)
n += n2
Expand Down Expand Up @@ -1109,6 +1176,9 @@ func (d *Decoder) allocPtrIfNil(v *reflect.Value) error {
}
if isNil {
vet := v.Type().Elem()
if err := d.TrackOutputBytes(int64(vet.Size())); err != nil {
return err
}
v.Set(reflect.New(vet))
}
return nil
Expand Down Expand Up @@ -1182,3 +1252,38 @@ func (d *Decoder) InputLen() (int, bool) {
}
return d.l.Len(), true
}

// Sentinel errors for memory tracking — using errors.New instead of
// fmt.Errorf keeps the function small enough for the Go compiler to inline.
// Callers wrap these with fmt.Errorf to add context.
var (
ErrMemoryLimitExceeded = errors.New("memory limit exceeded")
ErrNegativeTrackingSize = errors.New("negative tracking size")
)

// TrackOutputBytes adds size to the cumulative decoded output byte count and
// returns an error if MaxMemoryBytes has been exceeded. Generated and
// reflection-based decoders call this before each allocation (array element,
// union arm, optional field, opaque data, string) to bound memory
// amplification from untrusted input.
func (d *Decoder) TrackOutputBytes(size int64) error {
if d.maxMemoryBytes <= 0 {
return nil
}
if size < 0 {
return ErrNegativeTrackingSize
}
d.memoryBytes += size
if d.memoryBytes > d.maxMemoryBytes || d.memoryBytes < 0 {
return ErrMemoryLimitExceeded
}
Comment thread
tamirms marked this conversation as resolved.
return nil
}

// TrackOutputBytesOf is a generic helper that calls TrackOutputBytes with
// unsafe.Sizeof(*new(T)), which the compiler resolves to a constant at
// compile time. This keeps the unsafe import contained in go-xdr —
// generated code calls this function without needing to import unsafe.
func TrackOutputBytesOf[T any](d *Decoder) error {
return d.TrackOutputBytes(int64(unsafe.Sizeof(*new(T))))
Comment thread
tamirms marked this conversation as resolved.
}
75 changes: 75 additions & 0 deletions xdr3/decode_limits_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
package xdr

import (
"bytes"
"encoding/binary"
"errors"
"testing"
"unsafe"
)

// wideStruct has a large in-memory footprint (256 bytes) relative to its
// minimal XDR wire representation.
type wideStruct struct {
F0, F1, F2, F3, F4, F5, F6, F7 int64
F8, F9, F10, F11, F12, F13, F14, F15 int64
F16, F17, F18, F19, F20, F21, F22, F23 int64
F24, F25, F26, F27, F28, F29, F30, F31 int64
}

// makeArrayPayload creates an XDR-encoded variable-length array header
// followed by zero-filled element data.
func makeArrayPayload(payloadSize int) []byte {
payload := make([]byte, payloadSize)
declaredLen := uint32(payloadSize - 4)
binary.BigEndian.PutUint32(payload[0:4], declaredLen)
return payload
}

func TestMaxMemoryBytes(t *testing.T) {
payloadSize := 100000
payload := makeArrayPayload(payloadSize)
structSize := int64(unsafe.Sizeof(wideStruct{})) // 256
Comment thread
tamirms marked this conversation as resolved.

t.Run("unlimited", func(t *testing.T) {
var result []wideStruct
reader := bytes.NewReader(payload)
_, err := UnmarshalWithOptions(reader, &result, DecodeOptions{})
if len(result) == 0 {
t.Errorf("expected some decoded elements with no limit, err=%v", err)
}
})
Comment thread
tamirms marked this conversation as resolved.

t.Run("exceeded", func(t *testing.T) {
budget := int64(256) * structSize // 65536 bytes
var result []wideStruct
reader := bytes.NewReader(payload)
_, err := UnmarshalWithOptions(reader, &result, DecodeOptions{
MaxMemoryBytes: budget,
})
if err == nil {
t.Error("expected error when output byte limit exceeded")
}
maxExpected := int(budget/structSize) + 1
if len(result) > maxExpected {
t.Errorf("decoded %d elements, expected at most %d", len(result), maxExpected)
}
})

t.Run("not_reached", func(t *testing.T) {
// Budget larger than what the payload can produce — decode
// should succeed without hitting the limit.
budget := int64(3000) * structSize
var result []wideStruct
reader := bytes.NewReader(payload)
_, err := UnmarshalWithOptions(reader, &result, DecodeOptions{
MaxMemoryBytes: budget,
})
if errors.Is(err, ErrMemoryLimitExceeded) {
t.Errorf("expected budget not to be exceeded, got %v", err)
}
if len(result) == 0 {
t.Errorf("expected some decoded elements before hitting end of input, err=%v", err)
}
})
Comment thread
tamirms marked this conversation as resolved.
}
Loading