From c06e38a5141af9ec5630edb232a72a215a401df7 Mon Sep 17 00:00:00 2001 From: Kimball Thurston Date: Sat, 2 Mar 2024 13:30:50 +1300 Subject: [PATCH 1/4] fix sporadic UB in dwa Signed-off-by: Kimball Thurston --- src/lib/OpenEXRCore/internal_dwa_compressor.h | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/lib/OpenEXRCore/internal_dwa_compressor.h b/src/lib/OpenEXRCore/internal_dwa_compressor.h index 729b0d7425..3e986950b4 100644 --- a/src/lib/OpenEXRCore/internal_dwa_compressor.h +++ b/src/lib/OpenEXRCore/internal_dwa_compressor.h @@ -1699,12 +1699,23 @@ DwaCompressor_setupChannelData (DwaCompressor* me) cd->planarUncRle[0] = cd->planarUncBuffer; cd->planarUncRleEnd[0] = cd->planarUncRle[0]; - for (int byte = 1; byte < curc->bytes_per_element; ++byte) + if (!cd->planarUncBuffer) { - cd->planarUncRle[byte] = - cd->planarUncRle[byte - 1] + curc->width * curc->height; + for (int byte = 1; byte < curc->bytes_per_element; ++byte) + { + cd->planarUncRle[byte] = 0; + cd->planarUncRleEnd[byte] = 0; + } + } + else + { + for (int byte = 1; byte < curc->bytes_per_element; ++byte) + { + cd->planarUncRle[byte] = + cd->planarUncRle[byte - 1] + curc->width * curc->height; - cd->planarUncRleEnd[byte] = cd->planarUncRle[byte]; + cd->planarUncRleEnd[byte] = cd->planarUncRle[byte]; + } } cd->planarUncType = (exr_pixel_type_t) curc->data_type; From 5bf41de1c7a762de6aa75640ab2eb90e72de2535 Mon Sep 17 00:00:00 2001 From: Kimball Thurston Date: Sat, 2 Mar 2024 13:33:16 +1300 Subject: [PATCH 2/4] Allow partial chunk tables This will fix #1642, re-enabling partial chunk tables during reconstruction (at least for the last part read). Correctly handles reconstructing tables where discovered tiles are shifted from expected indices Signed-off-by: Kimball Thurston --- src/lib/OpenEXRCore/chunk.c | 43 ++++++++++++++++++++++++++++--------- 1 file changed, 33 insertions(+), 10 deletions(-) diff --git a/src/lib/OpenEXRCore/chunk.c b/src/lib/OpenEXRCore/chunk.c index 89f582dc92..c3d9df4d30 100644 --- a/src/lib/OpenEXRCore/chunk.c +++ b/src/lib/OpenEXRCore/chunk.c @@ -454,10 +454,12 @@ reconstruct_chunk_table ( uint64_t* chunktable) { exr_result_t rv = EXR_ERR_SUCCESS; + exr_result_t firstfailrv = EXR_ERR_SUCCESS; uint64_t offset_start, chunk_start, max_offset; uint64_t* curctable; const struct _internal_exr_part* curpart = NULL; int found_ci, computed_ci, partnum = 0; + size_t chunkbytes; curpart = ctxt->parts[ctxt->num_parts - 1]; offset_start = curpart->chunk_table_offset; @@ -492,6 +494,11 @@ reconstruct_chunk_table ( if (rv != EXR_ERR_SUCCESS) return rv; } + chunkbytes = (size_t)part->chunk_count * sizeof(uint64_t); + curctable = (uint64_t*) ctxt->alloc_fn (chunkbytes); + if (!curctable) return EXR_ERR_OUT_OF_MEMORY; + + memset (curctable, 0, chunkbytes); for (int ci = 0; ci < part->chunk_count; ++ci) { if (chunktable[ci] >= offset_start && chunktable[ci] < max_offset) @@ -503,21 +510,34 @@ reconstruct_chunk_table ( if (part->lineorder == EXR_LINEORDER_DECREASING_Y) computed_ci = part->chunk_count - (ci + 1); found_ci = computed_ci; + rv = read_and_validate_chunk_leader ( ctxt, part, partnum, chunk_start, &found_ci, &offset_start); - if (rv != EXR_ERR_SUCCESS) return rv; + if (rv != EXR_ERR_SUCCESS) + { + chunk_start = 0; + if (firstfailrv == EXR_ERR_SUCCESS) firstfailrv = rv; + } // scanlines can be more strict about the ordering if (part->storage_mode == EXR_STORAGE_SCANLINE || part->storage_mode == EXR_STORAGE_DEEP_SCANLINE) { - if (computed_ci != found_ci) return EXR_ERR_BAD_CHUNK_LEADER; + if (computed_ci != found_ci) + { + chunk_start = 0; + if (firstfailrv == EXR_ERR_SUCCESS) + firstfailrv = EXR_ERR_BAD_CHUNK_LEADER; + } } - chunktable[found_ci] = chunk_start; + if (curctable[found_ci] == 0) + curctable[found_ci] = chunk_start; } + memcpy (chunktable, curctable, chunkbytes); + ctxt->free_fn (curctable); - return rv; + return firstfailrv; } static exr_result_t @@ -566,10 +586,9 @@ extract_chunk_table ( if (rv != EXR_ERR_SUCCESS) { ctxt->free_fn (ctable); - return rv; + ctable = (uint64_t *) UINTPTR_MAX; } - - if (!ctxt->disable_chunk_reconstruct) + else if (!ctxt->disable_chunk_reconstruct) { // could convert table all at once, but need to check if the // file is incomplete (i.e. crashed during write and didn't @@ -592,13 +611,17 @@ extract_chunk_table ( rv = reconstruct_chunk_table (ctxt, part, ctable); if (rv != EXR_ERR_SUCCESS) { - ctxt->free_fn (ctable); - ctable = (uint64_t*) UINTPTR_MAX; if (ctxt->strict_header) - return ctxt->report_error ( + { + ctxt->free_fn (ctable); + ctable = (uint64_t *) UINTPTR_MAX; + rv = ctxt->report_error ( ctxt, EXR_ERR_BAD_CHUNK_LEADER, "Incomplete / corrupt chunk table, unable to reconstruct"); + } + else + rv = EXR_ERR_SUCCESS; } } } From 15d449454b8539ed71a9040eb6e8b3a22a7ab310 Mon Sep 17 00:00:00 2001 From: Kimball Thurston Date: Sat, 2 Mar 2024 15:02:50 +1300 Subject: [PATCH 3/4] Avoid OOB write When reconstructing chunk table with corrupt chunks, avoid writing out of bounds Signed-off-by: Kimball Thurston --- src/lib/OpenEXRCore/chunk.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/lib/OpenEXRCore/chunk.c b/src/lib/OpenEXRCore/chunk.c index c3d9df4d30..987408fdac 100644 --- a/src/lib/OpenEXRCore/chunk.c +++ b/src/lib/OpenEXRCore/chunk.c @@ -531,8 +531,11 @@ reconstruct_chunk_table ( } } - if (curctable[found_ci] == 0) - curctable[found_ci] = chunk_start; + if (found_ci >= 0 && found_ci < part->chunk_count) + { + if (curctable[found_ci] == 0) + curctable[found_ci] = chunk_start; + } } memcpy (chunktable, curctable, chunkbytes); ctxt->free_fn (curctable); From 9df25dbcf1faaeef2a69795df73a31242c37cb9a Mon Sep 17 00:00:00 2001 From: Kimball Thurston Date: Sat, 2 Mar 2024 15:03:28 +1300 Subject: [PATCH 4/4] avoid undefined behavior unaligned reads are UB, use (__builtin_) memcpy Signed-off-by: Kimball Thurston --- src/lib/OpenEXRCore/internal_huf.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/lib/OpenEXRCore/internal_huf.c b/src/lib/OpenEXRCore/internal_huf.c index b6c7b0077c..74d7124cd6 100644 --- a/src/lib/OpenEXRCore/internal_huf.c +++ b/src/lib/OpenEXRCore/internal_huf.c @@ -1007,13 +1007,23 @@ readUInt (const uint8_t* b) #ifdef __APPLE__ # include -# define READ64(c) OSSwapInt64 (*(const uint64_t*) (c)) +# define SWAP64(c) OSSwapInt64 (c) #elif defined(linux) # include -# define READ64(c) bswap_64 (*(const uint64_t*) (c)) +# define SWAP64(c) bswap_64 (c) #elif defined(_MSC_VER) # include -# define READ64(c) _byteswap_uint64 (*(const uint64_t*) (c)) +# define SWAP64(c) _byteswap_uint64 (c) +#endif + +#ifdef SWAP64 +static inline uint64_t READ64(const uint8_t *src) +{ + uint64_t v; + // unaligned reads are UB + memcpy (&v, src, sizeof(uint64_t)); + return SWAP64 (v); +} #else # define READ64(c) \ ((uint64_t) (c)[0] << 56) | ((uint64_t) (c)[1] << 48) | \