search

golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
124.39k stars 17.71k forks source link

math/big: Is it necessary to limit the value of the prec parameter in some methods, or to add explicit hints? #50817

Open And-ZJ opened 2 years ago

And-ZJ commented 2 years ago

What version of Go are you using (go version)?

$ go version

go version go1.17.5 windows/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env

not relevant

What did you do?

An error was reported in issue #50699 that ran out of memory due to unexpected integer overflow.

Note that some methods in the math/big module may cause memory exhaustion if the user passes an too large prec parameter.

For example:

func (x *Rat) FloatString(prec int) string. See in ratconv.go.

func (x *Float) Text(format byte, prec int) string. See in ftoa.go.

But some methods, such as func (z *Float) SetPrec(prec uint) *Float (See in float.go), can limit the prec parameter to MaxPrec .

Users may mistakenly assume that all these methods already provide some parameter protection internally.

So, do we need explicit maximum protection in these methods as well, or do we need explicit hints in the method documentation? For example, "passing in an excessively large prec parameter may cause memory exhaustion".

Note in particular the document The prec value is ignored for the'b' and'p' formats. (See in ftoa.go ), but in fact an excessively large prec parameter causes memory exhaustion, despite the use of b or p formats.This document may not be very appropriate.

A previous discussion can be found in this comments.

Some Tests:

func Test1(t *testing.T) {
    f := new(Float).SetInt64(12)
    f.Text('b', math.MaxInt32*100) // too large prec with b format.
    // out of memory
}
func Test2(t *testing.T) {
    f := new(Float).SetInt64(12)
    f.Text('f', math.MaxInt32*100) // too large prec with f format.
    // out of memory
}
func Test3(t *testing.T) {
    x := new(Rat).SetFloat64(0.5)
    x.FloatString(math.MaxInt32*100) 
    // timeout
}

What did you expect to see?

If this behavior is allowed, it might be better to add documentation to prompt users to protect the parameters.

If this behavior is not allowed, the method should limit the parameter value internally.

What did you see instead?

runtime: VirtualAlloc of 214748364800 bytes failed with errno=1455
fatal error: out of memory
mknyszek commented 2 years ago

CC @golang/security