Skip to content
This repository has been archived by the owner on Apr 2, 2023. It is now read-only.

panic: runtime error: index out of range in stereoProcessIntensityShort #23

Closed
gy741 opened this issue Feb 6, 2018 · 12 comments
Closed
Labels

Comments

@gy741
Copy link

gy741 commented Feb 6, 2018

Hello.

I found a index out of range bug in go-mp3.

Please confirm.

Thanks.

reproduce code:

package mp3

import (
	"bytes"
	"testing"
)

type bytesReadCloser struct {
	*bytes.Reader
}

func (b *bytesReadCloser) Close() error {
	return nil
}

func TestFuzzing(t *testing.T) {
	inputs := []string{
		"\xff\xfb%S000000v000\x00\x010000" +
		"00000000000000000000" +
		"0000\xf4000000000000000" +
		"00000000000000000000" +
		"00000000000000000000" +
		"00000000000000000000",
		
	}
	for _, input := range inputs {
		b := &bytesReadCloser{bytes.NewReader([]byte(input))}
		_, _ = NewDecoder(b)
	}
}

Log

--- FAIL: TestFuzzing (0.00s)
panic: runtime error: index out of range [recovered]
	panic: runtime error: index out of range

goroutine 5 [running]:
testing.tRunner.func1(0xc42004c750)
	/usr/lib/go-1.8/src/testing/testing.go:622 +0x29d
panic(0x527020, 0x5e99f0)
	/usr/lib/go-1.8/src/runtime/panic.go:489 +0x2cf
github.com/hajimehoshi/go-mp3/internal/frame.(*Frame).stereoProcessIntensityShort(0xc4200b6000, 0x0, 0x5)
	/home/karas/go/src/github.com/hajimehoshi/go-mp3/internal/frame/frame.go:337 +0x293
github.com/hajimehoshi/go-mp3/internal/frame.(*Frame).stereo(0xc4200b6000, 0x0)
	/home/karas/go/src/github.com/hajimehoshi/go-mp3/internal/frame/frame.go:398 +0x3b3
github.com/hajimehoshi/go-mp3/internal/frame.(*Frame).Decode(0xc4200b6000, 0xc4200149c0, 0x0, 0x0)
	/home/karas/go/src/github.com/hajimehoshi/go-mp3/internal/frame/frame.go:126 +0x119
github.com/hajimehoshi/go-mp3.(*Decoder).readFrame(0xc420018a20, 0x0, 0x0)
	/home/karas/go/src/github.com/hajimehoshi/go-mp3/decode.go:52 +0x17f
github.com/hajimehoshi/go-mp3.NewDecoder(0x5d8ca0, 0xc42000c098, 0x78, 0xc42005a980, 0x78)
	/home/karas/go/src/github.com/hajimehoshi/go-mp3/decode.go:207 +0xc0
github.com/hajimehoshi/go-mp3.TestFuzzing(0xc42004c750)
	/home/karas/go/src/github.com/hajimehoshi/go-mp3/fuzzing_test.go:29 +0x126
testing.tRunner(0xc42004c750, 0x554a40)
	/usr/lib/go-1.8/src/testing/testing.go:657 +0x96
created by testing.(*T).Run
	/usr/lib/go-1.8/src/testing/testing.go:697 +0x2ca
exit status 2
FAIL	github.com/hajimehoshi/go-mp3	0.006s
@lieff
Copy link

lieff commented Feb 6, 2018

@gy741 Can you test https://github.com/tosone/minimp3 with same fuzzer?

@gy741
Copy link
Author

gy741 commented Feb 7, 2018

@lieff

Yes, A little modification is necessary, but it is possible.

@hajimehoshi hajimehoshi added the bug label Feb 7, 2018
@hajimehoshi
Copy link
Owner

@lieff As the decoding part is written in C in minimp3, the fuzzing tests might not crash even it causes out of range error?

@lieff
Copy link

lieff commented Feb 7, 2018

If binding part guaranteed range check and safe data & len passing, then should not crash on any data.

@hajimehoshi
Copy link
Owner

OK but how about, not only crashes, but also silent errors? Sorry if I'm wrong, but on C side, there are no boundary checks, right? Is it possible to guarantee that there is no such boundary error on C side?

@lieff
Copy link

lieff commented Feb 7, 2018

C side have boundary check within passed mp3_bytes range, so if memory within this range available everything should be ok, otherwise it's a bug.

@hajimehoshi
Copy link
Owner

hajimehoshi commented Feb 7, 2018

I mean, for example, is there a boundary check for tabindex at https://github.com/tosone/minimp3/blob/master/minimp3.h#L779 ? In the past fuzzing test for this go-mp3, various boundary errors have been found.

@lieff
Copy link

lieff commented Feb 7, 2018

Yes, all boundaries should be fine (as I think), I've ask for fuzzing test to double check that. This code relatively young, so, errors still possible.

@hajimehoshi
Copy link
Owner

Thank you!

@gy741
Copy link
Author

gy741 commented Feb 7, 2018

@lieff

I proceeded to fuzzing about 10 hours.

I did not find a bug.

@dgryski
Copy link

dgryski commented Feb 8, 2018

@lieff To fuzz the C mp3 library, you'll want to use either http://lcamtuf.coredump.cx/afl/ or libfuzzer ( https://github.com/google/fuzzer-test-suite/blob/master/tutorial/libFuzzerTutorial.md . I can help you set this up if you're interested.

@lieff
Copy link

lieff commented Feb 8, 2018

@dgryski Thanks) I will try this.

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

No branches or pull requests

4 participants