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:
parent
ea57b9aa2c
commit
f94da15120
|
@ -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
|
||||||
|
|
|
@ -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;
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in New Issue