hajimehoshi / go-mp3

An MP3 decoder in pure Go
Apache License 2.0
746 stars 80 forks source link

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

Closed gy741 closed 6 years ago

gy741 commented 6 years ago

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 commented 6 years ago

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

gy741 commented 6 years ago

@lieff

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

hajimehoshi commented 6 years ago

@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 commented 6 years ago

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

hajimehoshi commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

Thank you!

gy741 commented 6 years ago

@lieff

I proceeded to fuzzing about 10 hours.

I did not find a bug.

dgryski commented 6 years ago

@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 commented 6 years ago

@dgryski Thanks) I will try this.