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

some vulnerability - 0x03 an out-of-bound vulnerability in readTextWithDescrFrame function #79

Closed
Jayl1n opened this issue Nov 19, 2020 · 7 comments

Comments

@Jayl1n
Copy link

Jayl1n commented Nov 19, 2020

This is the third vulnerability in id3v2frames.go

In readTextWithDescrFrame function, you don't check the size of b , program will happen panic when the size of b is 2 or less than 2 .

testcase 8eff69ad26a59a05ec11e38f3ca6c592f08dcc54.zip

panic: runtime error: slice bounds out of range [:3] with capacity 2

goroutine 1 [running]:
github.com/dhowden/tag.readTextWithDescrFrame(0xc0000da038, 0x3, 0x3, 0x101, 0x122b1a0, 0x0, 0x0)
        /Users/jaylin/go/pkg/mod/github.com/dhowden/tag@v0.0.0-20200828214007-46e57f75dbfc/id3v2frames.go:460 +0x46e
github.com/dhowden/tag.readID3v2Frames(0x114d680, 0xc0000d8000, 0x17, 0xc0000dc000, 0xc0000d8000, 0x0, 0xb)
        /Users/jaylin/go/pkg/mod/github.com/dhowden/tag@v0.0.0-20200828214007-46e57f75dbfc/id3v2.go:364 +0x90b
github.com/dhowden/tag.ReadID3v2Tags(0x114daa0, 0xc0000d8000, 0x1, 0x0, 0x0, 0x0)
        /Users/jaylin/go/pkg/mod/github.com/dhowden/tag@v0.0.0-20200828214007-46e57f75dbfc/id3v2.go:428 +0xde
github.com/dhowden/tag.ReadFrom(0x114daa0, 0xc0000d8000, 0xc0000d6000, 0x17, 0x217, 0x0)
        /Users/jaylin/go/pkg/mod/github.com/dhowden/tag@v0.0.0-20200828214007-46e57f75dbfc/tag.go:52 +0x324
main.main()
        /Users/jaylin/GolandProjects/gofuzz_test/main.go:20 +0xb5
@dhowden
Copy link
Owner

dhowden commented Nov 19, 2020

Duplicate of #76

@dhowden dhowden marked this as a duplicate of #76 Nov 19, 2020
@dhowden dhowden closed this as completed Nov 19, 2020
@Jayl1n
Copy link
Author

Jayl1n commented Nov 20, 2020

If the array size is less than 2, it still panic in the latest commit, just like in the figure below

图片

图片

@dhowden

@dhowden
Copy link
Owner

dhowden commented Nov 20, 2020

Ah yes! Thanks. Happy to receive a pull request to fix :-).

@dhowden dhowden reopened this Nov 20, 2020
@Jayl1n
Copy link
Author

Jayl1n commented Nov 20, 2020

Ah yes! Thanks. Happy to receive a pull request to fix :-).

@dhowden I'm Sorry. My code is so terrible, but I can give you an advice if you don't have a better way to fix such bugs.

You could use recover() function in which caller to regains control of a panicking goroutine. see more detail

@dhowden
Copy link
Owner

dhowden commented Nov 20, 2020

Ah yes! Thanks. Happy to receive a pull request to fix :-).

@dhowden I'm Sorry. My code is so terrible, but I can give you an advice if you don't have a better way to fix such bugs.

No worries :-)

I will have a look now.

@dhowden
Copy link
Owner

dhowden commented Nov 20, 2020

Just to note: the library was built to read data from valid files (and making it conform to all the specs was bad enough, so I mostly ignored safety measures to trap bad files).. Using a fuzzer will likely find lots of issues like this!

If people are using this in production environments, would definitely recommend that they wrap all alls to the library with recover (as you suggest above) to make sure that a panic here does not bring down their entire process.

@wader
Copy link
Contributor

wader commented Nov 20, 2020

@Jayl1n @dhowden nice work! would be possible to add your fuzzing test code to the repo?

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

No branches or pull requests

3 participants