Skip to content

Commit

Permalink
Merge pull request from GHSA-3669-72x9-r9p3
Browse files Browse the repository at this point in the history
* fixes the security advisory by limiting the slice creation based on configurable maxSize

* address review comment
  • Loading branch information
bharat-rajani authored Jun 29, 2024
1 parent 180f71e commit cd59f2f
Show file tree
Hide file tree
Showing 2 changed files with 137 additions and 6 deletions.
18 changes: 17 additions & 1 deletion decoder.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,21 @@ import (
"strings"
)

const (
defaultMaxSize = 16000
)

// NewDecoder returns a new Decoder.
func NewDecoder() *Decoder {
return &Decoder{cache: newCache()}
return &Decoder{cache: newCache(), maxSize: defaultMaxSize}
}

// Decoder decodes values from a map[string][]string to a struct.
type Decoder struct {
cache *cache
zeroEmpty bool
ignoreUnknownKeys bool
maxSize int
}

// SetAliasTag changes the tag used to locate custom field aliases.
Expand Down Expand Up @@ -54,6 +59,13 @@ func (d *Decoder) IgnoreUnknownKeys(i bool) {
d.ignoreUnknownKeys = i
}

// MaxSize limits the size of slices for URL nested arrays or object arrays.
// Choose MaxSize carefully; large values may create many zero-value slice elements.
// Example: "items.100000=apple" would create a slice with 100,000 empty strings.
func (d *Decoder) MaxSize(size int) {
d.maxSize = size
}

// RegisterConverter registers a converter function for a custom type.
func (d *Decoder) RegisterConverter(value interface{}, converterFunc Converter) {
d.cache.registerConverter(value, converterFunc)
Expand Down Expand Up @@ -302,6 +314,10 @@ func (d *Decoder) decode(v reflect.Value, path string, parts []pathPart, values
// Slice of structs. Let's go recursive.
if len(parts) > 1 {
idx := parts[0].index
// a defensive check to avoid creating a large slice based on user input index
if idx > d.maxSize {
return fmt.Errorf("%v index %d is larger than the configured maxSize %d", v.Kind(), idx, d.maxSize)
}
if v.IsNil() || v.Len() < idx+1 {
value := reflect.MakeSlice(t, idx+1, idx+1)
if v.Len() < idx+1 {
Expand Down
125 changes: 120 additions & 5 deletions decoder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2063,7 +2063,7 @@ type S24 struct {

type S24e struct {
*S24
F2 string `schema:"F2"`
F2 string `schema:"F2"`
}

func TestUnmarshallToEmbeddedNoData(t *testing.T) {
Expand All @@ -2074,13 +2074,14 @@ func TestUnmarshallToEmbeddedNoData(t *testing.T) {
s := &S24e{}

decoder := NewDecoder()
err := decoder.Decode(s, data);
err := decoder.Decode(s, data)

expectedErr := `schema: invalid path "F3"`
if err.Error() != expectedErr {
t.Fatalf("got %q, want %q", err, expectedErr)
}
}

type S25ee struct {
F3 string `schema:"F3"`
}
Expand All @@ -2095,14 +2096,13 @@ type S25 struct {
F1 string `schema:"F1"`
}

func TestDoubleEmbedded(t *testing.T){
func TestDoubleEmbedded(t *testing.T) {
data := map[string][]string{
"F1": {"raw a"},
"F2": {"raw b"},
"F3": {"raw c"},
}


s := S25{}
decoder := NewDecoder()

Expand Down Expand Up @@ -2412,3 +2412,118 @@ func TestDefaultsAreNotSupportedForStructsAndStructSlices(t *testing.T) {
t.Errorf("decoding should fail with error msg %s got %q", expected, err)
}
}

func TestDecoder_MaxSize(t *testing.T) {
t.Parallel()

type Nested struct {
Val int
NestedValues []struct {
NVal int
}
}
type NestedSlices struct {
Values []Nested
}

testcases := []struct {
name string
maxSize int
decoderInput func() (dst NestedSlices, src map[string][]string)
expectedDecoded NestedSlices
expectedErr MultiError
}{
{
name: "no error on decoding under max size",
maxSize: 10,
decoderInput: func() (dst NestedSlices, src map[string][]string) {
return dst, map[string][]string{
"Values.1.Val": {"132"},
"Values.1.NestedValues.1.NVal": {"1"},
"Values.1.NestedValues.2.NVal": {"2"},
"Values.1.NestedValues.3.NVal": {"3"},
}
},
expectedDecoded: NestedSlices{
Values: []Nested{
{
Val: 0,
NestedValues: nil,
},
{
Val: 132, NestedValues: []struct{ NVal int }{
{NVal: 0},
{NVal: 1},
{NVal: 2},
{NVal: 3},
},
},
},
},
expectedErr: nil,
},
{
name: "error on decoding above max size",
maxSize: 1,
decoderInput: func() (dst NestedSlices, src map[string][]string) {
return dst, map[string][]string{
"Values.1.Val": {"132"},
"Values.1.NestedValues.1.NVal": {"1"},
"Values.1.NestedValues.2.NVal": {"2"},
"Values.1.NestedValues.3.NVal": {"3"},
}
},
expectedErr: MultiError{
"Values.1.NestedValues.2.NVal": errors.New("slice index 2 is larger than the configured maxSize 1"),
"Values.1.NestedValues.3.NVal": errors.New("slice index 3 is larger than the configured maxSize 1"),
},
},
}

for _, tc := range testcases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
dec := NewDecoder()
dec.MaxSize(tc.maxSize)
dst, src := tc.decoderInput()
err := dec.Decode(&dst, src)

if tc.expectedErr != nil {
var gotErr MultiError
if !errors.As(err, &gotErr) {
t.Errorf("decoder error is not of type %T", gotErr)
}
if !reflect.DeepEqual(gotErr, tc.expectedErr) {
t.Errorf("expected %v, got %v", tc.expectedErr, gotErr)
}
} else {
if !reflect.DeepEqual(dst, tc.expectedDecoded) {
t.Errorf("expected %v, got %v", tc.expectedDecoded, dst)
}
}
})
}
}

func TestDecoder_SetMaxSize(t *testing.T) {

t.Run("default maxsize should be equal to given constant", func(t *testing.T) {
t.Parallel()
dec := NewDecoder()
if !reflect.DeepEqual(dec.maxSize, defaultMaxSize) {
t.Errorf("unexpected default max size")
}
})

t.Run("configured maxsize should be set properly", func(t *testing.T) {
t.Parallel()
configuredMaxSize := 50
limitedMaxSizeDecoder := NewDecoder()
limitedMaxSizeDecoder.MaxSize(configuredMaxSize)
if !reflect.DeepEqual(limitedMaxSizeDecoder.maxSize, configuredMaxSize) {
t.Errorf("invalid decoder maxsize, expected: %d, got: %d",
configuredMaxSize, limitedMaxSizeDecoder.maxSize)
}
})
}

0 comments on commit cd59f2f

Please sign in to comment.