aboutsummaryrefslogtreecommitdiff
path: root/src/liblzma
diff options
context:
space:
mode:
authorLasse Collin <lasse.collin@tukaani.org>2022-09-09 13:51:57 +0300
committerLasse Collin <lasse.collin@tukaani.org>2022-09-09 13:51:57 +0300
commitf8ee61e74eb40600445fdb601c374d582e1e9c8a (patch)
tree7f2d7b0545b4824a91b3947b220b77987c791ba3 /src/liblzma
parentliblzma: lzma_index_append: Add missing integer overflow check. (diff)
downloadxz-f8ee61e74eb40600445fdb601c374d582e1e9c8a.tar.xz
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.
Diffstat (limited to 'src/liblzma')
-rw-r--r--src/liblzma/api/lzma/filter.h4
-rw-r--r--src/liblzma/common/filter_common.c18
2 files changed, 15 insertions, 7 deletions
diff --git a/src/liblzma/api/lzma/filter.h b/src/liblzma/api/lzma/filter.h
index 8c859314..04825c65 100644
--- a/src/liblzma/api/lzma/filter.h
+++ b/src/liblzma/api/lzma/filter.h
@@ -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.
*
* 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
* - LZMA_MEM_ERROR
diff --git a/src/liblzma/common/filter_common.c b/src/liblzma/common/filter_common.c
index 9ad5d5d8..590be730 100644
--- a/src/liblzma/common/filter_common.c
+++ b/src/liblzma/common/filter_common.c
@@ -122,12 +122,16 @@ static const struct {
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)
{
- if (src == NULL || dest == NULL)
+ if (src == NULL || real_dest == NULL)
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;
size_t 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.
- assert(i <= LZMA_FILTERS_MAX + 1);
+ assert(i < LZMA_FILTERS_MAX + 1);
dest[i].id = LZMA_VLI_UNKNOWN;
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;
error:
// Free the options which we have already allocated.
- while (i-- > 0) {
+ while (i-- > 0)
lzma_free(dest[i].options, allocator);
- dest[i].options = NULL;
- }
return ret;
}