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

[BUG]: slice bounds out of range at modules/git #20315

Closed
secsys-go opened this issue Jul 11, 2022 · 4 comments · Fixed by #21368
Closed

[BUG]: slice bounds out of range at modules/git #20315

secsys-go opened this issue Jul 11, 2022 · 4 comments · Fixed by #21368
Labels

Comments

@secsys-go
Copy link

Description

I tried to parse some data with ParseTreeEntries in modules/git, but it crashed instead of returning an error

package main
import "code.gitea.io/gitea/modules/git"
func main() {
	git.ParseTreeEntries([]byte("1006440000000000000000000000000000000000000000000000000000000000000000000000\t\n1006440000000000000000000000000000000000000000000000000000000000000000000000\t\n1006440000000000000000000000000000000000000000000000000000000000000000000000\t\n1006440000000000000000000000000000000000000000000000"))
}

Found by go-fuzz

Gitea Version

with git commit as ee769f7

Can you reproduce the bug on the Gitea demo site?

No

Log Gist

No response

Screenshots

No response

Git Version

No response

Operating System

Ubuntu

How are you running Gitea?

I test the API in Gitea through go-fuzz

Database

No response

@lunny
Copy link
Member

lunny commented Jul 11, 2022

So I assume you are using SHA256 but not SHA1 ?

@wxiaoguang
Copy link
Contributor

So I assume you are using SHA256 but not SHA1 ?

Since that's generated by a fuzz test, it doesn't need to be SHA256 or SHA1.

@Gusted
Copy link
Contributor

Gusted commented Jul 11, 2022

Fuzzing Gitea directly by calling it's function is risky and IMO incorrect, because you want to make sure that fuzz-data go trough all stages as a normal workflow would, so just fuzzing a specific function in modules/ is useless as there's a good chance that before that function is called within Gitea it already did go trough other checks, not all validation and sanity checks live in modules/ but can also lives at go-chi level or in the routers/ level. Well they do present an interesting case, unless it's actually "actionable" it isn't worth adding checks when they may already exist.


I had a quick look at the function and there are two references, one is for testing and the other reference is a helper function for parsing the raw stdout of git commands. Unless git decides to change this output or somehow be able to corrupt the output this panic cannot ever exist.

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Oct 7, 2022

-> Refactor parseTreeEntries, speed up tree list #21368

The minimal reproducible case is:

package main

import "code.gitea.io/gitea/modules/git"

func main() {
	// "runtime error: slice bounds out of range"
	ParseTreeEntries([]byte("100644 blob ea0d83c9081af9500ac9f804101b3fd0a5c293af"))
}

wxiaoguang added a commit that referenced this issue Oct 7, 2022
Close #20315 (fix the panic when parsing invalid input), Speed up #20231 (use ls-tree without size field)

Introduce ListEntriesRecursiveFast (ls-tree without size) and ListEntriesRecursiveWithSize (ls-tree with size)
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants