fix allocation of very large blocks

[whisk]

Description

Corrupt or malicious files may declare unreasonable large metadata blocks even if the actual data itself much smaller. Parser allocates buffers of the declared size when reading such files, and that may lead to out-of-memory errors.

Ways to reproduce

I added testcase TestVorbisCommentTooManyTagsOOM to demostrate slowness and/or OOM after several iterations. In this PR it will pass because of the fix.

Fix

I added upper thresholds for number of tags in comment, number of seek points and raw picture size. If that threshold is hit, parsing fails with a ErrDeclaredBlockTooBig type error.

Thresholds are loose, and should accomodate any practical use case I can imagine. However, it should be difficult to exploit them now.

[mewmew]

Thanks for the PR!

Glad to see you've made the FLAC parser a bit more resilient against malicious FLAC streams.

I've added a few comments to increase the threshold limits. I do believe you set quite reasonable limits, but lets up them a bit to ensure no valid use cases are limited by the thresholds.

Cheers,
Robin

[whisk]

hi @mewmew, thank you for the review. I've increased the limits as you suggested. I agree that it's important to not break existing valid use cases. I think that even with higher limits it's still difficult to exploit the lib, except maybe for some large scale high-throughput scenarios.

[mewmew]

Thanks @whisk! I've now merged the change :)

Wish you a lovely late summer and happy coding!

[mewmew]

Hmm, at least locally the test cases fail after merging this PR:

--- FAIL: TestEncodeRoundTrip (9.94s)
    --- FAIL: TestEncodeRoundTrip/testdata/flac-test-files/subset/48_-_Extremely_large_SEEKTABLE.flac (0.00s)
        enc_test.go:153: "testdata/flac-test-files/subset/48 - Extremely large SEEKTABLE.flac": unable to parse FLAC file; meta.parseSeekTable: declared block size is too big to allocate, number of seekpoints: 932067
    --- FAIL: TestEncodeRoundTrip/testdata/flac-test-files/subset/54_-_1000x_repeating_VORBISCOMMENT.flac (0.00s)
        enc_test.go:153: "testdata/flac-test-files/subset/54 - 1000x repeating VORBISCOMMENT.flac": unable to parse FLAC file; meta.Block.parseVorbisComment: declared block size is too big to allocate, tags number=20000
    --- FAIL: TestEncodeRoundTrip/testdata/flac-test-files/subset/55_-_file_48-53_combined.flac (0.00s)
        enc_test.go:153: "testdata/flac-test-files/subset/55 - file 48-53 combined.flac": unable to parse FLAC file; meta.parseSeekTable: declared block size is too big to allocate, number of seekpoints: 932067
--- FAIL: TestEncodeAnalysisFixed (18.62s)
    --- FAIL: TestEncodeAnalysisFixed/testdata/flac-test-files/subset/48_-_Extremely_large_SEEKTABLE.flac (0.00s)
        enc_test.go:264: "testdata/flac-test-files/subset/48 - Extremely large SEEKTABLE.flac": unable to parse FLAC file; meta.parseSeekTable: declared block size is too big to allocate, number of seekpoints: 932067
    --- FAIL: TestEncodeAnalysisFixed/testdata/flac-test-files/subset/54_-_1000x_repeating_VORBISCOMMENT.flac (0.00s)
        enc_test.go:264: "testdata/flac-test-files/subset/54 - 1000x repeating VORBISCOMMENT.flac": unable to parse FLAC file; meta.Block.parseVorbisComment: declared block size is too big to allocate, tags number=20000
    --- FAIL: TestEncodeAnalysisFixed/testdata/flac-test-files/subset/55_-_file_48-53_combined.flac (0.00s)
        enc_test.go:264: "testdata/flac-test-files/subset/55 - file 48-53 combined.flac": unable to parse FLAC file; meta.parseSeekTable: declared block size is too big to allocate, number of seekpoints: 932067
--- FAIL: TestDecode (0.29s)
    --- FAIL: TestDecode/newSeek/testdata/flac-test-files/subset/48_-_Extremely_large_SEEKTABLE.flac (0.00s)
        flac_test.go:190: meta.parseSeekTable: declared block size is too big to allocate, number of seekpoints: 932067
    --- FAIL: TestDecode/parse/testdata/flac-test-files/subset/48_-_Extremely_large_SEEKTABLE.flac (0.00s)
        flac_test.go:190: meta.parseSeekTable: declared block size is too big to allocate, number of seekpoints: 932067
    --- FAIL: TestDecode/newSeek/testdata/flac-test-files/subset/54_-_1000x_repeating_VORBISCOMMENT.flac (0.00s)
        flac_test.go:190: meta.Block.parseVorbisComment: declared block size is too big to allocate, tags number=20000
    --- FAIL: TestDecode/parse/testdata/flac-test-files/subset/54_-_1000x_repeating_VORBISCOMMENT.flac (0.00s)
        flac_test.go:190: meta.Block.parseVorbisComment: declared block size is too big to allocate, tags number=20000
    --- FAIL: TestDecode/newSeek/testdata/flac-test-files/subset/55_-_file_48-53_combined.flac (0.00s)
        flac_test.go:190: meta.parseSeekTable: declared block size is too big to allocate, number of seekpoints: 932067
    --- FAIL: TestDecode/parse/testdata/flac-test-files/subset/55_-_file_48-53_combined.flac (0.00s)
        flac_test.go:190: meta.parseSeekTable: declared block size is too big to allocate, number of seekpoints: 932067
FAIL
FAIL	github.com/mewkiz/flac	28.885s
ok  	github.com/mewkiz/flac/frame	10.856s
ok  	github.com/mewkiz/flac/internal/bits	0.008s
ok  	github.com/mewkiz/flac/internal/bufseekio	0.002s
?   	github.com/mewkiz/flac/internal/hashutil	[no test files]
ok  	github.com/mewkiz/flac/internal/hashutil/crc16	0.002s
ok  	github.com/mewkiz/flac/internal/hashutil/crc8	0.002s
?   	github.com/mewkiz/flac/internal/ioutilx	[no test files]
?   	github.com/mewkiz/flac/internal/utf8	[no test files]
ok  	github.com/mewkiz/flac/meta	0.003s
FAIL

@whisk, do you have the chance to take a look? And perhaps adjust the thresholds if needed or make the error for these test cases a valid success?

[whisk]

@mewmew, I see that "testdata" is missing some of the test files, for example:
testdata/flac-test-files/subset/48 - Extremely large SEEKTABLE.flac
testdata/flac-test-files/subset/51 - Extremely large VORBISCOMMENT.flac
The whole testdata/flac-test-files dir is empty, but others may be missing too. I didn't find a way to get those missing files 😢

TestEncodeRoundTrip skips test cases if test files do not exist, this is likely the cause it was overlooked.

In any case, it's still not a problem to further increase the limits for the number of seekpoints and number of tags.
1 million seekpoints is 18 MB (a single SeekPoint is 18 bytes if I'm not mistaken), and 20k tags is 80 KB — much less than 128 MB for a picture.

Please see #78 for this.

[mewmew]

The whole testdata/flac-test-files dir is empty, but others may be missing too. I didn't find a way to get those missing files 😢

The testdata/flac-test-files directory is a git submodule. So just run git submodule update --init --recursive to get those test files : )