Stack / heap overflows with libiamf test vectors

[jzern]

@ 5f8911cab86df546cc18bceccd817f44482b027f

The following fail when running under ASan:

• tests/test_000000_3.iamf
• tests/test_000050.iamf
• tests/test_000055.iamf

To reproduce add -DCMAKE_C_FLAGS=-fsanitize=address -DCMAKE_CXX_FLAGS=-fsanitize=address -DCMAKE_BUILD_TYPE=Debug to the CMake command lines for libiamf and iamfplayer. See the attached log for details. log.txt.gz

[jwcullen]

@jzern - To have a full history of this it would also be helpful to know what version of libiamf the .iamf files are from. Is it safe to assume you used libiamff88e17c?

[jzern]

Good point. Yes, libiamf was at that commit.

[jzern]

Using libiamf @ f88e17c56f6d1f1bbcc54474ac8a676743993a31 and iac @ f48a9398c41ee9ccfa0c5e5a1c36b0b2471c40f5, there are still AddressSanitizer warnings.

For example:

==2139942==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x611000000140 at pc 0x7f705e159fb6 bp 0x7ffc41aed650 sp 0x7ffc41aed648
SUMMARY: AddressSanitizer: heap-buffer-overflow src/iamf_dec/bitstream.c:195 in readi16le
==2140193==ERROR: AddressSanitizer: memcpy-param-overlap: memory ranges [0x7f47b0aba8f8,0x7f47b0aba9f0) and [0x7f47b0aba900, 0x7f47b0aba9f8) overlap
SUMMARY: AddressSanitizer: memcpy-param-overlap ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:827 in __interceptor_memcpy

log.txt.gz

[yilun-zhangs]

Hello~

ASAN is a good method to find possible memory overflow issue. Thanks for this sharing, we are checking it.

[jzern]

Thanks for having a look. After ASan is cleared, I suggest -fsanitize=undefined, -fsanitize=integer and -fsanitize=cfi. I don't think there's any threading going on so -fsanitize=thread could be skipped.

[jzern]

I forgot to add -fsanitize=memory. You'll want to use -DCMAKE_C_COMPILER=clang.

[jingbo-marquis]

tests/test_000000_3.iamf: the last frame obu only has 64 samples, not 128. it causes heap-buffer-overflow issue. test_000000_3.textproto: codec_config_metadata { codec_config_id: 200 codec_config { codec_id: 0x6970636d # "ipcm" num_samples_per_frame: 128 roll_distance: 0 decoder_config_lpcm { sample_format_flags: LPCM_LITTLE_ENDIAN sample_size: 16 sample_rate: 16000 } } }

[jwcullen]

See my comment in https://github.com/AOMediaCodec/libiamf/issues/41. Part of the purpose "is_valid: false" test vectors is to test how the decoder handles input that does not fully comply with the spec.

It's possible someone uses the decoder in a production environment and the input bitstream is invalid or corrupt. Ideally this would be detected and rejected gracefully instead of potentially ending up with undefined behavior.

[jzern]

Beyond the invalid test vectors produced, this decoder should undergo fuzzing to make sure it's resilient. One option is https://github.com/google/oss-fuzz.

[yilun-zhangs]

Hello~

Thanks for sharing these options, we will try them at our side. We will consider to let decoder be robust to handle invalid cases.

Thanks.

[yilun-zhangs]

I have installed clang. And set cmake option: SET(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -O3 -fsanitize=address -fsanitize=undefined -fsanitize=integer -fsanitize=cfi -fsanitize=memory")

Then error is like following:

clang: error: invalid argument '-fsanitize=cfi' only allowed with '-flto' clang: error: invalid argument '-fsanitize=address' not allowed with '-fsanitize=memory' clang: error: invalid argument '-fsanitize=cfi' only allowed with '-fvisibility='

If only set cmake option: SET(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -O3 -fsanitize=undefined -fsanitize=integer -fsanitize=memory")

There is no error.

My Cmake version is

3.24.3

[jzern]

Just set one sanitizer at a time. memory, undefined and integer should work without other changes.

@whnam0303 were new changes pushed to address the issue? If not I'd like to keep this bug open.

[yilun-zhangs]

@jzern Please keep this bug open~

[yilun-zhangs]

I only set one sanitizer : -fsanitize=cfi Still meet following errors...

-- The C compiler identification is Clang 6.0.0 -- The CXX compiler identification is GNU 7.5.0 -- Detecting C compiler ABI info -- Detecting C compiler ABI info - failed -- Check for working C compiler: /usr/bin/clang -- Check for working C compiler: /usr/bin/clang - broken CMake Error at /usr/local/share/cmake-3.24/Modules/CMakeTestCCompiler.cmake:69 (message): The C compiler

"/usr/bin/clang"

is not able to compile a simple test program.

[jzern]

Please keep this bug open~

I don't have permission to reopen this. @whnam0303 please reopen the bug if the ASan issues haven't been addressed. If they have, then new bugs can be opened for each sanitizer.

@yilun-zhangs you can link to issues in your commit message (issue # NNN, etc.) or attach them to the PR manually. This helps track progress.

https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue

I only set one sanitizer : -fsanitize=cfi

cfi is a bit trickier. Try undefined or integer first. Contrary to what I mentioned before, memory may be difficult to use since it requires system libraries to have been compiled with it.

[yilun-zhangs]

This is good idea, I have created a new issue, #15 Later, new commit code will be linked to this Issue.

Thanks.

[jzern]

For the record, I think e37f8177bd34330cb2ffe4e218c22957a0e296c5 attempted to address some issues. @ 5862c22b825a038503553d0a1eb57ff1ead879dc I still see some issues with gcc 12.2.0 and -fsanitize=address; clang-14 fails to compile the source.

For example:

==> tests/test_000000_3.iamf <==
=================================================================
==1253784==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x611000000140 at pc 0x7febded5a056 bp 0x7ffe7647f370 sp 0x7ffe7647f368
READ of size 1 at 0x611000000140 thread T0
    #0 0x7febded5a055 in readi16le src/iamf_dec/bitstream.c:206
    #1 0x7febded3bbc0 in iamf_pcm_decode src/iamf_dec/pcm/IAMF_pcm_decoder.c:108

See the attached log. The runs were done with test/tools/iamfplayer/iamfplayer -o2 -s11. log.txt.gz