liblzma: Fix invalid free() after memory allocation failure.

The bug was in the single-threaded .xz Stream encoder
in the code that is used for both re-initialization and for
lzma_filters_update(). To trigger it, an application had
to either re-initialize an existing encoder instance with
lzma_stream_encoder() or use lzma_filters_update(), and
then one of the 1-4 tiny allocations in lzma_filters_copy()
(called from stream_encoder_update()) must fail. An error
was correctly reported but the encoder state was corrupted.

This is related to the recent fix in
f8ee61e74e which is good but
it wasn't enough to fix the main problem in stream_encoder.c.
This commit is contained in:
Lasse Collin 2022-11-23 21:26:21 +02:00
parent cafd6dc397
commit 10430fbf38
1 changed files with 31 additions and 8 deletions

View File

@ -233,6 +233,13 @@ stream_encoder_update(void *coder_ptr, const lzma_allocator *allocator,
const lzma_filter *reversed_filters) const lzma_filter *reversed_filters)
{ {
lzma_stream_coder *coder = coder_ptr; lzma_stream_coder *coder = coder_ptr;
lzma_ret ret;
// Make a copy to a temporary buffer first. This way it is easier
// to keep the encoder state unchanged if an error occurs with
// lzma_filters_copy().
lzma_filter temp[LZMA_FILTERS_MAX + 1];
return_if_error(lzma_filters_copy(filters, temp, allocator));
if (coder->sequence <= SEQ_BLOCK_INIT) { if (coder->sequence <= SEQ_BLOCK_INIT) {
// There is no incomplete Block waiting to be finished, // There is no incomplete Block waiting to be finished,
@ -240,31 +247,47 @@ stream_encoder_update(void *coder_ptr, const lzma_allocator *allocator,
// trying to initialize the Block encoder with the new // trying to initialize the Block encoder with the new
// chain. This way we detect if the chain is valid. // chain. This way we detect if the chain is valid.
coder->block_encoder_is_initialized = false; coder->block_encoder_is_initialized = false;
coder->block_options.filters = (lzma_filter *)(filters); coder->block_options.filters = temp;
const lzma_ret ret = block_encoder_init(coder, allocator); ret = block_encoder_init(coder, allocator);
coder->block_options.filters = coder->filters; coder->block_options.filters = coder->filters;
if (ret != LZMA_OK) if (ret != LZMA_OK)
return ret; goto error;
coder->block_encoder_is_initialized = true; coder->block_encoder_is_initialized = true;
} else if (coder->sequence <= SEQ_BLOCK_ENCODE) { } else if (coder->sequence <= SEQ_BLOCK_ENCODE) {
// We are in the middle of a Block. Try to update only // We are in the middle of a Block. Try to update only
// the filter-specific options. // the filter-specific options.
return_if_error(coder->block_encoder.update( ret = coder->block_encoder.update(
coder->block_encoder.coder, allocator, coder->block_encoder.coder, allocator,
filters, reversed_filters)); filters, reversed_filters);
if (ret != LZMA_OK)
goto error;
} else { } else {
// Trying to update the filter chain when we are already // Trying to update the filter chain when we are already
// encoding Index or Stream Footer. // encoding Index or Stream Footer.
return LZMA_PROG_ERROR; ret = LZMA_PROG_ERROR;
goto error;
} }
// Free the copy of the old chain and make a copy of the new chain. // Free the options of the old chain.
for (size_t i = 0; coder->filters[i].id != LZMA_VLI_UNKNOWN; ++i) for (size_t i = 0; coder->filters[i].id != LZMA_VLI_UNKNOWN; ++i)
lzma_free(coder->filters[i].options, allocator); lzma_free(coder->filters[i].options, allocator);
return lzma_filters_copy(filters, coder->filters, allocator); // Copy the new filter chain in place.
size_t j = 0;
do {
coder->filters[j].id = temp[j].id;
coder->filters[j].options = temp[j].options;
} while (temp[j++].id != LZMA_VLI_UNKNOWN);
return LZMA_OK;
error:
for (size_t i = 0; temp[i].id != LZMA_VLI_UNKNOWN; ++i)
lzma_free(temp[i].options, allocator);
return ret;
} }