panic: runtime error: slice bounds out of range in gaad.freq_derived

[gy741]

Hello.

I found a slice bounds out of range bug in gaad.

Please confirm.

Thanks.

reproduce code:

package gaad

import (
	"testing"
)

func TestFuzzCrashers(t *testing.T) {

	var crashers = []string{
			"\xff\xf1L0000\xda000000000000" +
	        "0\xa8\xc3000n\xe40000\xbe\x99\xb10\x96\xfc\xea0" +
	        "\xddڀ\x000}\xf70000g?\xf10\x00\x00000" +
	        "\xb200\x8c\xe7Z00\xf7\xca0@\xcb00\xcb#<\xdb1" +
	        "\xde~0\xfc]",
	}

	for _, f := range crashers {
		ParseADTS([]byte(f))
	}
}

Crash Log:

--- FAIL: TestFuzzCrashers (0.00s)
panic: runtime error: slice bounds out of range [recovered]
	panic: runtime error: slice bounds out of range

goroutine 17 [running]:
testing.tRunner.func1(0xc42004d2b0)
	/usr/lib/go-1.8/src/testing/testing.go:622 +0x29d
panic(0x5337a0, 0x5fbe20)
	/usr/lib/go-1.8/src/runtime/panic.go:489 +0x2cf
github.com/Comcast/gaad.freq_derived(0xc420084380, 0xc401021806)
	/home/karas/go/src/github.com/Comcast/gaad/aacsbrtables.go:250 +0x487
github.com/Comcast/gaad.derive_sbr_tables(0xc420084380, 0x60102050c00, 0xc4200b5350, 0x0)
	/home/karas/go/src/github.com/Comcast/gaad/aacsbrtables.go:88 +0x1ac
github.com/Comcast/gaad.(*ADTS).sbr_extension_data(0xc42004d380, 0x7, 0x102, 0x0, 0x0, 0xc420019800, 0x40ef01)
	/home/karas/go/src/github.com/Comcast/gaad/aacparser.go:1903 +0xeb
github.com/Comcast/gaad.(*ADTS).extension_payload(0xc42004d380, 0x7, 0x4f0002, 0x0, 0x0, 0x0, 0x0)
	/home/karas/go/src/github.com/Comcast/gaad/aacparser.go:1740 +0x3d4
github.com/Comcast/gaad.(*ADTS).fill_element(0xc42004d380, 0x2, 0x6, 0x0, 0x0)
	/home/karas/go/src/github.com/Comcast/gaad/aacparser.go:1293 +0xbf
github.com/Comcast/gaad.(*ADTS).raw_data_block(0xc42004d380, 0x0, 0x0)
	/home/karas/go/src/github.com/Comcast/gaad/aacparser.go:1020 +0x269
github.com/Comcast/gaad.(*ADTS).adts_frame(0xc42004d380, 0xc420019800, 0x55)
	/home/karas/go/src/github.com/Comcast/gaad/aacparser.go:796 +0x91
github.com/Comcast/gaad.ParseADTS(0xc420017b00, 0x55, 0x60, 0xc420017b00, 0x55, 0x60)
	/home/karas/go/src/github.com/Comcast/gaad/aacparser.go:773 +0xbe
github.com/Comcast/gaad.TestFuzzCrashers(0xc42004d2b0)
	/home/karas/go/src/github.com/Comcast/gaad/fuzzer_test.go:18 +0x90
testing.tRunner(0xc42004d2b0, 0x564830)
	/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/Comcast/gaad	0.005s

[rwhitworth]

Go 1.18 makes this fairly easy to reproduce on the current 'master' branch

func FuzzGaad(f *testing.F) {
        testcases := []string{"0123456789abcdef", "ff"}
        for _, tc := range testcases {
                f.Add(tc)
        }
        f.Fuzz(func(t *testing.T, orig string) {
                buf, _ := hex.DecodeString(orig)
                adts, err := gaad.ParseADTS(buf)
                _ = adts
                _ = err
        })
}

Output:

root@debian11:~/fuzz# go test -fuzz=Gaad
fuzz: elapsed: 0s, gathering baseline coverage: 0/2 completed
fuzz: elapsed: 0s, gathering baseline coverage: 2/2 completed, now fuzzing with 2 workers
fuzz: minimizing 32-byte failing input file
fuzz: elapsed: 0s, minimizing
--- FAIL: FuzzGaad (0.01s)
    --- FAIL: FuzzGaad (0.00s)
        testing.go:1349: panic: runtime error: index out of range [0] with length 0
            goroutine 22 [running]:
            runtime/debug.Stack()
                /usr/local/go/src/runtime/debug/stack.go:24 +0x90
            testing.tRunner.func1()
                /usr/local/go/src/testing/testing.go:1349 +0x1f2
            panic({0x5e7720, 0xc0000160f0})
                /usr/local/go/src/runtime/panic.go:838 +0x207
            github.com/Comcast/gaad/bitreader.NewBitReader(...)
                /root/go/pkg/mod/github.com/!comcast/[email protected]/bitreader/bitreader.go:30
            github.com/Comcast/gaad.ParseADTS({0xc0000141b8, 0x0, 0x8})
                /root/go/pkg/mod/github.com/!comcast/[email protected]/aacparser.go:774 +0xed
            fuzz1.FuzzGaad.func1(0x0?, {0x72cde0?, 0x0?})
                /root/fuzz1/main_test.go:30 +0x65
            reflect.Value.call({0x5c6e20?, 0x607fa0?, 0x13?}, {0x5f8e7c, 0x4}, {0xc000068810, 0x2, 0x2?})
                /usr/local/go/src/reflect/value.go:556 +0x845
            reflect.Value.Call({0x5c6e20?, 0x607fa0?, 0x514?}, {0xc000068810, 0x2, 0x2})
                /usr/local/go/src/reflect/value.go:339 +0xbf
            testing.(*F).Fuzz.func1.1(0x0?)
                /usr/local/go/src/testing/fuzz.go:337 +0x231
            testing.tRunner(0xc000091860, 0xc0000b41b0)
                /usr/local/go/src/testing/testing.go:1439 +0x102
            created by testing.(*F).Fuzz.func1
                /usr/local/go/src/testing/fuzz.go:324 +0x5b8


    Failing input written to testdata/fuzz/FuzzGaad/771e938e4458e983a736261a702e27c7a414fd660a15b63034f290b146d2f217
    To re-run:
    go test -run=FuzzGaad/771e938e4458e983a736261a702e27c7a414fd660a15b63034f290b146d2f217
FAIL
exit status 1
FAIL    fuzz1   0.014s

[LimitlessEarth]

I'll take a look in the next couple of days!

[LimitlessEarth]

Got it reproduced.

[LimitlessEarth]

I have not forgotten about this.

What I noticed while goofing with the fuzzer is that this library was written like the spec and like a C library. This pattern is repeated everywhere... While I acknowledge it is not bit safe it should still be fairly robust. Comcast has tens of thousands of streams with AAC streams and we have taken care of the bugs we have found from our production. I don't really currently have the time to rewrite this to make it have fewer assumptions of this nature.