From 3454230543d5d50ef95a5a89c15b2206c70a4dca Mon Sep 17 00:00:00 2001 From: Richard Geldreich Date: Sat, 18 Jul 2026 16:47:58 -0400 Subject: [PATCH] KTX2: overflow-safe range checks and input bounds in ktx2_transcoder::init The KTX2 level and supercompression-global-data (SGD) offset/length fields are full 64-bit values read from the file. init() validated them with "offset + length > m_data_size", where the addition can wrap around and incorrectly pass for out-of-range values. Rewrite both checks as "offset > m_data_size || length > (uint64_t)m_data_size - offset", which cannot overflow (offset is confirmed <= m_data_size before the subtraction, so it can't underflow either). The result is identical to the original for every in-range input. Also in init(): - Reject a level whose byte_length is 0. This case already logged an error but fell through without returning false. - Add KTX2_MAX_SUPPORTED_LAYER_COUNT (65535) and reject files whose layer_count exceeds it, mirroring KTX2_MAX_SUPPORTED_LEVEL_COUNT, so layer_count * face_count * level_count stays bounded. read_slice_offset_len_global_data(): compute image_count in 64-bit, verify m_sgd_byte_length == image_count * descriptor_size before sizing the descriptor array, and use try_resize() (returns false on failure) instead of resize() (which aborts the process) so an out-of-range count fails cleanly instead of attempting an oversized allocation. Also compute image_count in 64-bit in decompress_etc1s_global_data(). Valid files are handled exactly as before. Co-Authored-By: Claude Opus 4.8 --- transcoder/basisu_transcoder.cpp | 65 +++++++++++++++++++++----------- transcoder/basisu_transcoder.h | 1 + 2 files changed, 43 insertions(+), 23 deletions(-) diff --git a/transcoder/basisu_transcoder.cpp b/transcoder/basisu_transcoder.cpp index 2f96b52..721d298 100644 --- a/transcoder/basisu_transcoder.cpp +++ b/transcoder/basisu_transcoder.cpp @@ -19880,6 +19880,13 @@ namespace basist return false; } + // Sanity check the layer count. m_layer_count is a full attacker-controlled 32-bit value; bound it so that layer_count * face_count * level_count (used to size the per-image descriptor arrays) stays small. (layer_count==0 means a non-array texture, i.e. one layer.) + if (m_header.m_layer_count > KTX2_MAX_SUPPORTED_LAYER_COUNT) + { + BASISU_DEVEL_ERROR("ktx2_transcoder::init: Too many layers or file is corrupted or invalid\n"); + return false; + } + if ((m_header.m_supercompression_scheme == KTX2_SS_UASTC_HDR_6x6I) || (m_header.m_supercompression_scheme == KTX2_SS_XUASTC_LDR) || (m_header.m_supercompression_scheme == KTX2_SS_XUBC7)) @@ -19898,13 +19905,17 @@ namespace basist (m_header.m_supercompression_scheme == KTX2_SS_XUASTC_LDR) || (m_header.m_supercompression_scheme == KTX2_SS_XUBC7)) { - if (m_header.m_sgd_byte_offset.get_uint64() < sizeof(ktx2_header)) + const uint64_t sgd_byte_offset = m_header.m_sgd_byte_offset.get_uint64(); + const uint64_t sgd_byte_length = m_header.m_sgd_byte_length.get_uint64(); + + if (sgd_byte_offset < sizeof(ktx2_header)) { BASISU_DEVEL_ERROR("ktx2_transcoder::init: Supercompression global data offset is too low\n"); return false; } - if (m_header.m_sgd_byte_offset.get_uint64() + m_header.m_sgd_byte_length.get_uint64() > m_data_size) + // Overflow-safe range check: m_sgd_byte_offset/m_sgd_byte_length are attacker-controlled full 64-bit values, so "offset + length > m_data_size" can wrap past 2^64 and pass. Compare against the remaining bytes instead (sgd_byte_offset <= m_data_size is checked first, so the subtraction can't underflow). + if ((sgd_byte_offset > m_data_size) || (sgd_byte_length > (uint64_t)m_data_size - sgd_byte_offset)) { BASISU_DEVEL_ERROR("ktx2_transcoder::init: Supercompression global data offset and/or length is too high\n"); return false; @@ -19930,18 +19941,23 @@ namespace basist // Sanity check the level offsets and byte sizes for (uint32_t i = 0; i < m_levels.size(); i++) { - if (m_levels[i].m_byte_offset.get_uint64() < sizeof(ktx2_header)) + const uint64_t level_byte_offset = m_levels[i].m_byte_offset.get_uint64(); + const uint64_t level_byte_length = m_levels[i].m_byte_length.get_uint64(); + + if (level_byte_offset < sizeof(ktx2_header)) { BASISU_DEVEL_ERROR("ktx2_transcoder::init: Invalid level offset (too low)\n"); return false; } - if (!m_levels[i].m_byte_length.get_uint64()) + if (!level_byte_length) { BASISU_DEVEL_ERROR("ktx2_transcoder::init: Invalid level byte length\n"); + return false; } - if ((m_levels[i].m_byte_offset.get_uint64() + m_levels[i].m_byte_length.get_uint64()) > m_data_size) + // Overflow-safe range check: m_byte_offset/m_byte_length are attacker-controlled full 64-bit values, so "offset + length > m_data_size" can wrap past 2^64 and pass, leaving an arbitrarily large offset that later flows into pointer arithmetic (transcode_image_level/decompress_level_data). Compare against the remaining bytes instead (level_byte_offset <= m_data_size is checked first, so the subtraction can't underflow). + if ((level_byte_offset > m_data_size) || (level_byte_length > (uint64_t)m_data_size - level_byte_offset)) { BASISU_DEVEL_ERROR("ktx2_transcoder::init: Invalid level offset and/or length\n"); return false; @@ -20943,39 +20959,41 @@ namespace basist bool ktx2_transcoder::read_slice_offset_len_global_data(bool read_std_structs) { - const uint32_t image_count = basisu::maximum(m_header.m_layer_count, 1) * m_header.m_face_count * m_header.m_level_count; + // m_layer_count (<= KTX2_MAX_SUPPORTED_LAYER_COUNT), m_face_count (1 or 6) and m_level_count (<= KTX2_MAX_SUPPORTED_LEVEL_COUNT) were all validated in init(), so this product is small. Computed in 64-bit anyway as defense in depth against a future limit increase overflowing a uint32_t (or a 32-bit size_t). + const uint64_t image_count = (uint64_t)basisu::maximum(m_header.m_layer_count, 1) * m_header.m_face_count * m_header.m_level_count; assert(image_count); - + const uint8_t* pSrc = m_pData + m_header.m_sgd_byte_offset.get_uint64(); - m_slice_offset_len_descs.resize(image_count); + // The descriptor array must exactly fill the supercompression global data, whose offset/length were already sanity checked in init() to be inside the file and after the KTX2 header. + // Validate the expected byte size BEFORE allocating, so an attacker can't force a huge speculative allocation (e.g. via a bogus layer count) that would abort the process. Because m_sgd_byte_length is bounded by the file size, this also bounds image_count. + const uint64_t desc_size = read_std_structs ? sizeof(ktx2_slice_offset_len_desc_std) : sizeof(ktx2_slice_offset_len_desc_orig); + + if (m_header.m_sgd_byte_length.get_uint64() != image_count * desc_size) + { + BASISU_DEVEL_ERROR("ktx2_transcoder::read_slice_offset_len_global_data: Invalid global data length\n"); + return false; + } + + if (!m_slice_offset_len_descs.try_resize(image_count)) + { + BASISU_DEVEL_ERROR("ktx2_transcoder::read_slice_offset_len_global_data: Out of memory\n"); + return false; + } - // SGD offset/length already sanity checked to be inside the file and after the KTX2 header. if (read_std_structs) { - if (m_header.m_sgd_byte_length.get_uint64() != image_count * sizeof(ktx2_slice_offset_len_desc_std)) - { - BASISU_DEVEL_ERROR("ktx2_transcoder::read_slice_offset_len_global_data: Invalid global data length (0)\n"); - return false; - } - const ktx2_slice_offset_len_desc_std* pSrc_std_descs = reinterpret_cast(pSrc); for (uint32_t i = 0; i < image_count; i++) { - // TODO: Ignoring type (profile) for now, but we could check it + // TODO: Ignoring type (profile) for now, but we could check it m_slice_offset_len_descs[i].m_slice_byte_offset = pSrc_std_descs[i].m_slice_byte_offset; m_slice_offset_len_descs[i].m_slice_byte_length = pSrc_std_descs[i].m_slice_byte_length; } } else { - if (m_header.m_sgd_byte_length.get_uint64() != image_count * sizeof(ktx2_slice_offset_len_desc_orig)) - { - BASISU_DEVEL_ERROR("ktx2_transcoder::read_slice_offset_len_global_data: Invalid global data length (1)\n"); - return false; - } - memcpy((void*)m_slice_offset_len_descs.data(), pSrc, sizeof(ktx2_slice_offset_len_desc_orig) * image_count); } @@ -21001,7 +21019,8 @@ namespace basist //for (uint32_t i = 1; i < m_header.m_level_count; i++) // layer_pixel_depth += basisu::maximum(m_header.m_pixel_depth >> i, 1); - const uint32_t image_count = basisu::maximum(m_header.m_layer_count, 1) * m_header.m_face_count * m_header.m_level_count; + // m_layer_count (<= KTX2_MAX_SUPPORTED_LAYER_COUNT), m_face_count (1 or 6) and m_level_count (<= KTX2_MAX_SUPPORTED_LEVEL_COUNT) were all validated in init(), so this product is small. Computed in 64-bit anyway as defense in depth against a future limit increase overflowing a uint32_t (or a 32-bit size_t in the size check below). + const uint64_t image_count = (uint64_t)basisu::maximum(m_header.m_layer_count, 1) * m_header.m_face_count * m_header.m_level_count; assert(image_count); const uint8_t* pSrc = m_pData + m_header.m_sgd_byte_offset.get_uint64(); diff --git a/transcoder/basisu_transcoder.h b/transcoder/basisu_transcoder.h index a317613..4a961cd 100644 --- a/transcoder/basisu_transcoder.h +++ b/transcoder/basisu_transcoder.h @@ -1097,6 +1097,7 @@ namespace basist const uint32_t KTX2_IMAGE_IS_P_FRAME = 2; const uint32_t KTX2_UASTC_BLOCK_SIZE = 16; // also the block size for UASTC_HDR const uint32_t KTX2_MAX_SUPPORTED_LEVEL_COUNT = 16; // this is an implementation specific constraint and can be increased + const uint32_t KTX2_MAX_SUPPORTED_LAYER_COUNT = 65535; // this is an implementation specific constraint and can be increased // The KTX2 transfer functions supported by KTX2 const uint32_t KTX2_KHR_DF_TRANSFER_LINEAR = 1;