Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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 this issue Jan 26, 2022 · 1 comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@And-ZJ
Copy link
Contributor

And-ZJ commented Jan 26, 2022

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
Copy link
Contributor

CC @golang/security

@mknyszek mknyszek added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 26, 2022
@mknyszek mknyszek added this to the Backlog milestone Jan 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

2 participants