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

False positive - go/allocation-size-overflow #16368

Open
natefinch opened this issue Apr 30, 2024 · 2 comments
Open

False positive - go/allocation-size-overflow #16368

natefinch opened this issue Apr 30, 2024 · 2 comments

Comments

@natefinch
Copy link

Description of the false positive

This rule fires when you make a slice from the length of another slice and add a small constant size to it. It's literally impossible to overflow MaxInt on a 64 bit machine in today's world in this case.

Code samples or links to source code

// simplified for this example
func extend(input []byte) []byte {
	return make([]byte, len(input)+1)
}

** More Discussion **

In order for this to overflow, the input slice would need to be of size MaxInt. That's approximately 9 million terabytes if we're talking []byte on a 64 bit machine. The largest machine on azure right now has 12 TB of RAM. Even if we assume RAM size doubles every year, no machine will have 9 million terabytes of RAM for at least 20 years. So, you can't have a slice of anything except an empty struct that is anywhere near MaxInt length.

Until that time, it's literally impossible to have a slice of bytes with a length that is MaxInt-1 on a 64 bit machine. You'd run out of memory loooong before you had to worry about overflowing the int in the make() call.

Is there a way this check could be changed so that it won't trigger if you're getting the length off some other slice? Or are we worried about 32bit architectures, because that does not seem like something we should worry about at GitHub.

I don't really know much about how CodeQL works or what it can infer, but I'm open to other ways to avoid this check.

@ginsbach
Copy link
Contributor

ginsbach commented May 2, 2024

Thank you for reporting this.

Our current policy is to not give priority to false positives, but we acknowledge this and will track it internally for future consideration.

@smowton
Copy link
Contributor

smowton commented May 7, 2024

FYI if len returned an int64 this would already have been squashed; the alert is raised because len returns int which as you say can be of any size >= 32 bits depending on architecture, and we don't know where this code is intended to run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants