root/xz
1
0
Fork 0
mirror of https://git.tukaani.org/xz.git synced 2025-04-15 04:00:50 +00:00
Code Issues Releases Wiki Activity

liblzma: mt dec: Don't free the input buffer too early (CVE-2025-31115)

Browse Source 
The input buffer must be valid as long as the main thread is writing
to the worker-specific input buffer. Fix it by making the worker
thread not free the buffer on errors and not return the worker thread to
the pool. The input buffer will be freed when threads_end() is called.

With invalid input, the bug could at least result in a crash. The
effects include heap use after free and writing to an address based
on the null pointer plus an offset.

The bug has been there since the first committed version of the threaded
decoder and thus affects versions from 5.3.3alpha to 5.8.0.

As the commit message in 4cce3e27f529 says, I had made significant
changes on top of Sebastian's patch. This bug was indeed introduced
by my changes; it wasn't in Sebastian's version.

Thanks to Harri K. Koskinen for discovering and reporting this issue.

Fixes: 4cce3e27f529 ("liblzma: Add threaded .xz decompressor.")
Reported-by: Harri K. Koskinen <x64nop@nannu.org>
Reviewed-by: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
Thanks-to: Sam James <sam@gentoo.org>
(cherry picked from commit d5a2ffe41bb77b918a8c96084885d4dbe4bf6480)
This commit is contained in:
Lasse Collin 2025-04-03 14:34:42 +03:00
parent f74cf18ad0
commit 1b874b4f04
No known key found for this signature in database
GPG Key ID: 38EE757D69184620
1 changed files with 22 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
src/liblzma/common/stream_decoder_mt.c
View File

@ -435,8 +435,7 @@ next_loop_unlocked:
}
// Either we finished successfully (LZMA_STREAM_END) or an error
// occurred. Both cases are handled almost identically. The error
// case requires updating thr->coder->thread_error.
// occurred.
//
// The sizes are in the Block Header and the Block decoder
// checks that they match, thus we know these:
@ -444,16 +443,30 @@ next_loop_unlocked:
assert(ret != LZMA_STREAM_END
|| thr->out_pos == thr->block_options.uncompressed_size);
// Free the input buffer. Don't update in_size as we need
// it later to update thr->coder->mem_in_use.
lzma_free(thr->in, thr->allocator);
thr->in = NULL;
mythread_sync(thr->mutex) {
// Block decoder ensures this, but do a sanity check anyway
// because thr->in_filled < thr->in_size means that the main
// thread is still writing to thr->in.
if (ret == LZMA_STREAM_END && thr->in_filled != thr->in_size) {
assert(0);
ret = LZMA_PROG_ERROR;
}
if (thr->state != THR_EXIT)
thr->state = THR_IDLE;
}
// Free the input buffer. Don't update in_size as we need
// it later to update thr->coder->mem_in_use.
//
// This step is skipped if an error occurred because the main thread
// might still be writing to thr->in. The memory will be freed after
// threads_end() sets thr->state = THR_EXIT.
if (ret == LZMA_STREAM_END) {
lzma_free(thr->in, thr->allocator);
thr->in = NULL;
}
mythread_sync(thr->coder->mutex) {
// Move our progress info to the main thread.
thr->coder->progress_in += thr->in_pos;
@ -474,8 +487,8 @@ next_loop_unlocked:
thr->coder->thread_error = ret;
// Return the worker thread to the stack of available
// threads.
{
// threads only if no errors occurred.
if (ret == LZMA_STREAM_END) {
// Update memory usage counters.
thr->coder->mem_in_use -= thr->in_size;
thr->in_size = 0; // thr->in was freed above.