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
f8ee61e74eb40600445fdb601c374d582e1e9c8a which is good but
it wasn't enough to fix the main problem in stream_encoder.c.

1 files changed, 31 insertions, 8 deletions

diff --git a/src/liblzma/common/stream_encoder.c b/src/liblzma/common/stream_encoder.c
index 4dfe88cb..b15229c3 100644
--- a/src/liblzma/common/stream_encoder.c
+++ b/src/liblzma/common/stream_encoder.c
@@ -233,6 +233,13 @@ stream_encoder_update(void *coder_ptr, const lzma_allocator *allocator,
 		const lzma_filter *reversed_filters)
 {
 	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) {
 		// 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
 		// chain. This way we detect if the chain is valid.
 		coder->block_encoder_is_initialized = false;
-		coder->block_options.filters = (lzma_filter *)(filters);
-		const lzma_ret ret = block_encoder_init(coder, allocator);
+		coder->block_options.filters = temp;
+		ret = block_encoder_init(coder, allocator);
 		coder->block_options.filters = coder->filters;
 		if (ret != LZMA_OK)
-			return ret;
+			goto error;
 
 		coder->block_encoder_is_initialized = true;
 
 	} else if (coder->sequence <= SEQ_BLOCK_ENCODE) {
 		// We are in the middle of a Block. Try to update only
 		// the filter-specific options.
-		return_if_error(coder->block_encoder.update(
+		ret = coder->block_encoder.update(
 				coder->block_encoder.coder, allocator,
-				filters, reversed_filters));
+				filters, reversed_filters);
+		if (ret != LZMA_OK)
+			goto error;
 	} else {
 		// Trying to update the filter chain when we are already
 		// 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)
 		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;
 }