Starting an array at an arbitrary number is an opportunity for DoS attacks

[justinas]

I'm referring to this feature here:

Array honours the specified index. eg. if "Array[2]" is the only Array value passed down, it will be put at index 2; if array isn't big enough it will be expanded.

If a user submits a key like Array[1234567890], the application will allocate over 1G of memory just for this single request. Of course, no one is stopping users from putting an even higher number as an index.

This snippet demonstrates the problem:

package main

import (
    "log"
    "net/url"

    "github.com/go-playground/form"
)

type A struct {
    Array []int
}

func main() {
    decoder := form.NewDecoder()
    a := A{}
    err := decoder.Decode(&a, url.Values{"Array[1234567890]": {"42"}})
    if err != nil {
        log.Fatalln(err)
    }
    for {
    }
}

The RES usage of this application spikes to 1220MB.

[deankarn]

Although I do think it is a little outside of the scope of this library, I do agree if someone's endpoint were not secured, or a man in the middle attack were to cause this it could be a real issue.

I will look at adding a sane default array limit for decoder, say 10,000 or something (also allow it to be changed if more or less is desired) and return an error for that particular field if the limit is exceeded.

Let me know if you believe this would be sufficient, or you have any other ideas :)

[deankarn]

ok @justinas global array limit has been added to avoid/limit this potential issue.

now your program will output Field Namespace:Array ERROR:Array size of '1234567891' is larger than the maximum currently set on the decoder of '10000'. To increase this limit please see, SetMaxArraySize(size uint) by default.

and can adjust the limit by calling SetMaxArraySize(size uint)

Thanks for reporting 😄

please let me know if this satisfies your needs.

P.S. I was going to add individual field array limits, added via tags, however the library usage would become complicated quickly with arrays of arrays eg. [][]int specifying the limit for the inner array I would have to mimic some of the logic in my validator library with tags such as form:"maxlen=10,dive,maxlen=5". I may add this in the future, but for now this should provide enough protection.

[justinas]

Seems sensible. Thanks for the quick response!

[deankarn]

No thank you! @justinas I'm a huge proponent of open source, but it only works when people collaborate and contribute 😄