wild-addr-write found by fuzz

[ltx2018]

we found wild-addr-write by fuzzing flac-master:

==217==ERROR: AddressSanitizer: SEGV on unknown address 0xb6029a2c (pc 0x0822a2ae bp 0xffeb31e8 sp 0xffeb30a0 T0)
==217==The signal is caused by a WRITE memory access.
SCARINESS: 30 (wild-addr-write)
    #0 0x822a2ad in FLAC__bitwriter_write_raw_uint32_nocheck /src/flac/src/libFLAC/bitwriter.c
    #1 0x8229a42 in FLAC__bitwriter_write_raw_uint32 /src/flac/src/libFLAC/bitwriter.c:369:9
    #2 0x8218ec3 in FLAC__frame_add_header /src/flac/src/libFLAC/stream_encoder_framing.c:227:6
    #3 0x820557b in process_subframes_ /src/flac/src/libFLAC/stream_encoder.c:3365:7
    #4 0x81d940f in process_frame_ /src/flac/src/libFLAC/stream_encoder.c:3096:6
    #5 0x81f3770 in FLAC__stream_encoder_process_interleaved /src/flac/src/libFLAC/stream_encoder.c:2298:9
    #6 0x81bfa80 in FLAC::Encoder::Stream::process_interleaved(int const*, unsigned int) /src/flac/src/libFLAC++/stream_encoder.cpp:370:29
    #7 0x81ac167 in LLVMFuzzerTestOneInput /src/flac-fuzzers/fuzzer_encoder.cpp:141:46
    #8 0x80ac766 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned int) /src/llvm/projects/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:556:15
    #9 0x8098c13 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned int) /src/llvm/projects/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:292:6
    #10 0x809e318 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned int)) /src/llvm/projects/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:774:9
    #11 0x80c3167 in main /src/llvm/projects/compiler-rt/lib/fuzzer/FuzzerMain.cpp:19:10
    #12 0xf7539636 in __libc_start_main (/lib32/libc.so.6+0x18636)
    #13 0x8073c38 in _start (/out/flac/fuzzer_encoder+0x8073c38)

here is my debug info:
bw->buffer was realloc here

bitwriter_grow_ (bw=0xf5a00a90, bits_to_add=62914562) at bitwriter.c:128
128		if(new_buffer == 0)
(gdb) n
130		bw->buffer = new_buffer;
(gdb) l
125		FLAC__ASSERT(new_capacity >= bw->words + ((bw->bits + bits_to_add + FLAC__BITS_PER_WORD - 1) / FLAC__BITS_PER_WORD));
126
127		new_buffer = safe_realloc_mul_2op_(bw->buffer, sizeof(bwword), /*times*/new_capacity);
128		if(new_buffer == 0)
129			return false;
130		bw->buffer = new_buffer;
131		bw->capacity = new_capacity;
132		return true;
133	}
134
(gdb) p new_buffer
$1 = (bwword *) 0x7abd7800
(gdb) p new_capacity
$2 = 250956800

later, bw->buffer was freed but it's value NOT set to 0

156	static inline void *safe_realloc_(void *ptr, size_t size)
157	{
158		void *oldptr = ptr;
159		void *newptr = realloc(ptr, size);
160		if(size > 0 && newptr == 0)
161			free(oldptr);
162		return newptr;
(gdb) n
159		void *newptr = realloc(ptr, size);
(gdb) n
160		if(size > 0 && newptr == 0)
(gdb) p newptr
$4 = (void *) 0x0
(gdb) p size
$5 = 1006448640
(gdb) n
161			free(oldptr);
(gdb) p oldptr
$6 = (void *) 0x7abd7800
(gdb) n
162		return newptr;
(gdb) n
safe_realloc_mul_2op_ (ptr=0x7abd7800, size1=4, size2=251612160) at ../../include/share/alloc.h:206
206	}
(gdb) n
bitwriter_grow_ (bw=0xf5a00a90, bits_to_add=20971521) at bitwriter.c:128
128		if(new_buffer == 0)
(gdb) l
123		FLAC__ASSERT(0 == (new_capacity - bw->capacity) % FLAC__BITWRITER_DEFAULT_INCREMENT);
124		FLAC__ASSERT(new_capacity > bw->capacity);
125		FLAC__ASSERT(new_capacity >= bw->words + ((bw->bits + bits_to_add + FLAC__BITS_PER_WORD - 1) / FLAC__BITS_PER_WORD));
126
127		new_buffer = safe_realloc_mul_2op_(bw->buffer, sizeof(bwword), /*times*/new_capacity);
128		if(new_buffer == 0)
129			return false;
130		bw->buffer = new_buffer;
131		bw->capacity = new_capacity;
132		return true;
(gdb) p bw->buffer
$7 = (bwword *) 0x7abd7800
(gdb) p bw->capacity
$8 = 250956800
(gdb) n
129			return false

[ltx2018]

#214 a way to fix

[erikd]

How can this be triggered?

[ltx2018]

How can this be triggered?

i just run fuzzer_encoder(build by oss-fuzz) locally, here is my crash-corpus
crash-fuzzer_encoder.zip

[ltx2018]

How can this be triggered?

by the way, ARCHITECTURE is i386

[attritionorg]

How it is triggered is important for many reasons, not just debugging. It helps us understand if this can legitimately be triggered by a user and if so, represents a security risk.

[ltx2018]

How it is triggered is important for many reasons, not just debugging. It helps us understand if this can legitimately be triggered by a user and if so, represents a security risk.

Yeah, you can run fuzzer_encoder crash-fuzzer_encoder to reproduce(source code is oss-fuzz/fuzz-encoder.cc), as i have already mentioned before.

[erikd]

This is only triggered by submitting a specially crafted input to the encoder . It would be a far bigger issue if this was in the decoder.

[petterreinholdtsen]

[Erik de Castro Lopo]

This is only triggered by submitting a specially crafted input to the *encoder* . It would be a far bigger issue if this was in the decoder.

Absolutely. But given these web streaming days, even encoding might provide an avenue into the system. :)

…

-- Happy hacking Petter Reinholdtsen

[attritionorg]

Is there potential for more than a crash / denial of service?

[ktmf01]

@ltx2018 I've tried to reproduce this, but I seem to be bumping into this problem with clang/LLVM, which prevents me from building fuzzer_encoder on a 32-bit system.

Anyway, I have a hunch about what might be triggering the problem. Would you mind checking whether PR #252 solves this problem? I know #214 should already fix the problem, but I think #252 might be fixing the root cause. Please let me know.

[ktmf01]

@ltx2018 Okay, I finally got this one to trigger. #252 didn't fix it, neither did #251.

[ktmf01]

I believe this is fixed with the merge of #273. @ltx2018 could you check?

[ktmf01]

This is fixed with the merge of #419

[cebarks]

FYI, this flaw is being tracked by CVE-2020-22219.

[ktmf01]

I think the analysis of CVE-2020-22219 is wrong. The vulnerability described in CVE-2020-22219 is in libFLAC. However, this vulnerability is only exploitable when the program calling the library does not fulfill a key requirement of the library API: the calling program has to supply values that are out-of-bounds. This requirement is mentioned here, here and here.

This means that there is no way to exploit the vulnerability through the flac command line tool that is provided by the project: it does not violate this requirement, it only supplies values that are within bounds.

So, the vulnerability is only exploitable with a program calling libFLAC that is does not properly check whether inputs are bounded or not. That seems a pretty important omission in the current analysis.

[ktmf01]

On second thought, there is another way to exploit this, but that is very unreliable: when running out of memory. As FLAC uses only a tiny amount of memory and the failing realloc only allocates a small amount of memory, this is probably hard to turn into an exploit.

In short, this buffer overflow only happens after an unhandled memory allocation failure.

[mlichvar]

Doesn't the attacker control how much memory will the encoder need? With flac it causes a double free, but I guess other applications might do something with the encoder after the reallocation failed.

[ktmf01]

No, the encoding parameters bound how much memory is used, so an attacker cannot let libFLAC use an arbitrary amount of memory, except when the program allows for supplying invalid data. The problem here was that invalid input made the encoder go beyond those bounds. That is why #273 adds an explicit check that input is valid, #252 added a limit to the bitwriter size (which was increased in #378) and #419 fixed handling of the allocation failure.