liblzma: lzma_filters_copy: Keep dest[] unmodified if an error occurs.

lzma_stream_encoder() and lzma_stream_encoder_mt() always assumed
this. Before this patch, failing lzma_filters_copy() could result
in free(invalid_pointer) or invalid memory reads in stream_encoder.c
or stream_encoder_mt.c.

To trigger this, allocating memory for a filter options structure
has to fail. These are tiny allocations so in practice they very
rarely fail.

Certain badness in the filter chain array could also make
lzma_filters_copy() fail but both stream_encoder.c and
stream_encoder_mt.c validate the filter chain before
trying to copy it, so the crash cannot occur this way.
This commit is contained in:
Lasse Collin 2022-09-09 13:51:57 +03:00
parent 18d7facd38
commit f8ee61e74e
2 changed files with 15 additions and 7 deletions

View File

@ -108,7 +108,9 @@ extern LZMA_API(lzma_bool) lzma_filter_decoder_is_supported(lzma_vli id)
* need to be initialized by the caller in any way. * need to be initialized by the caller in any way.
* *
* If an error occurs, memory possibly already allocated by this function * If an error occurs, memory possibly already allocated by this function
* is always freed. * is always freed. liblzma versions older than 5.2.7 may modify the dest
* array and leave its contents in an undefined state if an error occurs.
* liblzma 5.2.7 and newer only modify the dest array when returning LZMA_OK.
* *
* \return - LZMA_OK * \return - LZMA_OK
* - LZMA_MEM_ERROR * - LZMA_MEM_ERROR

View File

@ -122,12 +122,16 @@ static const struct {
extern LZMA_API(lzma_ret) extern LZMA_API(lzma_ret)
lzma_filters_copy(const lzma_filter *src, lzma_filter *dest, lzma_filters_copy(const lzma_filter *src, lzma_filter *real_dest,
const lzma_allocator *allocator) const lzma_allocator *allocator)
{ {
if (src == NULL || dest == NULL) if (src == NULL || real_dest == NULL)
return LZMA_PROG_ERROR; return LZMA_PROG_ERROR;
// Use a temporary destination so that the real destination
// will never be modied if an error occurs.
lzma_filter dest[LZMA_FILTERS_MAX + 1];
lzma_ret ret; lzma_ret ret;
size_t i; size_t i;
for (i = 0; src[i].id != LZMA_VLI_UNKNOWN; ++i) { for (i = 0; src[i].id != LZMA_VLI_UNKNOWN; ++i) {
@ -173,18 +177,20 @@ lzma_filters_copy(const lzma_filter *src, lzma_filter *dest,
} }
// Terminate the filter array. // Terminate the filter array.
assert(i <= LZMA_FILTERS_MAX + 1); assert(i < LZMA_FILTERS_MAX + 1);
dest[i].id = LZMA_VLI_UNKNOWN; dest[i].id = LZMA_VLI_UNKNOWN;
dest[i].options = NULL; dest[i].options = NULL;
// Copy it to the caller-supplied array now that we know that
// no errors occurred.
memcpy(real_dest, dest, (i + 1) * sizeof(lzma_filter));
return LZMA_OK; return LZMA_OK;
error: error:
// Free the options which we have already allocated. // Free the options which we have already allocated.
while (i-- > 0) { while (i-- > 0)
lzma_free(dest[i].options, allocator); lzma_free(dest[i].options, allocator);
dest[i].options = NULL;
}
return ret; return ret;
} }