From a3740b5f713eb6a93981690fd52789a02ab8127d Mon Sep 17 00:00:00 2001 From: Scott Mansell Date: Sun, 26 Jul 2026 18:13:13 +1200 Subject: GCZ: Don't trust GetBlockCompressedSize It comes unverified from the file, and a maliciously crafted file could trigger not one, but two buffer overflows in the heap. --- Source/Core/DiscIO/CompressedBlob.cpp | 36 ++++++++++++++++++++++------------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/Source/Core/DiscIO/CompressedBlob.cpp b/Source/Core/DiscIO/CompressedBlob.cpp index 5c2ab84cdd..eae4d4ccaf 100644 --- a/Source/Core/DiscIO/CompressedBlob.cpp +++ b/Source/Core/DiscIO/CompressedBlob.cpp @@ -31,6 +31,8 @@ namespace DiscIO { +static constexpr u64 uncompressed_flag = 1ULL << 63; + bool IsGCZBlob(File::DirectIOFile& file); CompressedBlobReader::CompressedBlobReader(File::DirectIOFile file, std::string filename) @@ -80,9 +82,9 @@ std::unique_ptr CompressedBlobReader::CopyReader() const // IMPORTANT: Calling this function invalidates all earlier pointers gotten from this function. u64 CompressedBlobReader::GetBlockCompressedSize(u64 block_num) const { - u64 start = m_block_pointers[block_num]; + u64 start = m_block_pointers[block_num] & ~uncompressed_flag; if (block_num < m_header.num_blocks - 1) - return m_block_pointers[block_num + 1] - start; + return (m_block_pointers[block_num + 1] & ~uncompressed_flag) - start; else if (block_num == m_header.num_blocks - 1) return m_header.compressed_data_size - start; else @@ -93,21 +95,29 @@ u64 CompressedBlobReader::GetBlockCompressedSize(u64 block_num) const bool CompressedBlobReader::GetBlock(u64 block_num, u8* out_ptr) { bool uncompressed = false; - u32 comp_block_size = (u32)GetBlockCompressedSize(block_num); + u64 read_size = GetBlockCompressedSize(block_num); u64 offset = m_block_pointers[block_num] + m_data_offset; - if (offset & (1ULL << 63)) + if (offset & uncompressed_flag) { - if (comp_block_size != m_header.block_size) + if (read_size != m_header.block_size) + { ERROR_LOG_FMT(DISCIO, "Uncompressed block with wrong size"); + return false; + } uncompressed = true; - offset &= ~(1ULL << 63); + offset &= ~uncompressed_flag; + } + else + { + if (read_size > m_zlib_buffer.size()) + { + ERROR_LOG_FMT(DISCIO, "Compressed block is too large"); + return false; + } } - // clear unused part of zlib buffer. maybe this can be deleted when it works fully. - memset(&m_zlib_buffer[comp_block_size], 0, m_zlib_buffer.size() - comp_block_size); - - if (!m_file.OffsetRead(offset, m_zlib_buffer.data(), comp_block_size)) + if (!m_file.OffsetRead(offset, m_zlib_buffer.data(), read_size)) { ERROR_LOG_FMT(DISCIO, "The disc image \"{}\" is truncated, some of the data is missing.", m_file_name); @@ -115,7 +125,7 @@ bool CompressedBlobReader::GetBlock(u64 block_num, u8* out_ptr) } // First, check hash. - const u32 block_hash = Common::HashAdler32(m_zlib_buffer.data(), comp_block_size); + const u32 block_hash = Common::HashAdler32(m_zlib_buffer.data(), read_size); if (block_hash != m_hashes[block_num]) { ERROR_LOG_FMT(DISCIO, @@ -126,13 +136,13 @@ bool CompressedBlobReader::GetBlock(u64 block_num, u8* out_ptr) if (uncompressed) { - std::copy_n(m_zlib_buffer.begin(), comp_block_size, out_ptr); + std::copy_n(m_zlib_buffer.begin(), m_header.block_size, out_ptr); } else { z_stream z = {}; z.next_in = m_zlib_buffer.data(); - z.avail_in = comp_block_size; + z.avail_in = read_size; if (z.avail_in > m_header.block_size) { ERROR_LOG_FMT(DISCIO, "Compressed block size is larger than uncompressed block size"); -- cgit v1.2.3 From ffb0e2c59406c485a983098ad5bd85f8b3ae376e Mon Sep 17 00:00:00 2001 From: Scott Mansell Date: Sun, 26 Jul 2026 18:15:07 +1200 Subject: GCZ: Don't trust block_num either SectorReader::ReadChunk does do some validation on it, but it only checks against the original disc size (reported by the GCZ file). It has no idea how many blocks the header claimed the disc had. A maliciously crafted GCZ file could trigger read overflows off the end of the m_block_pointers/m_hashes arrays. --- Source/Core/DiscIO/CompressedBlob.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Source/Core/DiscIO/CompressedBlob.cpp b/Source/Core/DiscIO/CompressedBlob.cpp index eae4d4ccaf..036e139d0b 100644 --- a/Source/Core/DiscIO/CompressedBlob.cpp +++ b/Source/Core/DiscIO/CompressedBlob.cpp @@ -94,6 +94,9 @@ u64 CompressedBlobReader::GetBlockCompressedSize(u64 block_num) const bool CompressedBlobReader::GetBlock(u64 block_num, u8* out_ptr) { + if (block_num >= m_header.num_blocks) + return false; + bool uncompressed = false; u64 read_size = GetBlockCompressedSize(block_num); u64 offset = m_block_pointers[block_num] + m_data_offset; -- cgit v1.2.3 From feeb36bf07a187175232340e2e4d8a00d226fca0 Mon Sep 17 00:00:00 2001 From: Scott Mansell Date: Sun, 26 Jul 2026 20:20:08 +1200 Subject: GCZ: validate while loading --- Source/Core/DiscIO/CompressedBlob.cpp | 104 ++++++++++++++++++++++++++++++---- Source/Core/DiscIO/CompressedBlob.h | 25 ++++---- 2 files changed, 106 insertions(+), 23 deletions(-) diff --git a/Source/Core/DiscIO/CompressedBlob.cpp b/Source/Core/DiscIO/CompressedBlob.cpp index 036e139d0b..46573b6111 100644 --- a/Source/Core/DiscIO/CompressedBlob.cpp +++ b/Source/Core/DiscIO/CompressedBlob.cpp @@ -37,38 +37,78 @@ bool IsGCZBlob(File::DirectIOFile& file); CompressedBlobReader::CompressedBlobReader(File::DirectIOFile file, std::string filename) : m_file(std::move(file)), m_file_name(std::move(filename)) +{ + m_valid = Initialize(); +} + +bool CompressedBlobReader::Initialize() { m_file_size = m_file.GetSize(); m_file.Seek(0, File::SeekOrigin::Begin); - m_file.Read(Common::AsWritableU8Span(m_header)); + if (!m_file.Read(Common::AsWritableU8Span(m_header))) + return false; - SetSectorSize(m_header.block_size); + if (m_header.magic_cookie != GCZ_MAGIC) + return false; + + size_t block_pointers_size = m_header.num_blocks * sizeof(u64); + size_t hashes_size = m_header.num_blocks * sizeof(u32); + + size_t header_size = sizeof(CompressedBlobHeader) + block_pointers_size + hashes_size; + + // Basic sanity check for size before we start allocating + if (header_size > m_file_size) + { + ERROR_LOG_FMT(DISCIO, "Headers' size is larger than file size"); + return false; + } + + if ((header_size + m_header.compressed_data_size) > m_file_size) + { + ERROR_LOG_FMT(DISCIO, "Data size is larger than file size."); + return false; + } + + if (m_header.num_blocks == 0) + { + ERROR_LOG_FMT(DISCIO, "GCZ file has zero blocks"); + return false; + } // cache block pointers and hashes m_block_pointers.resize(m_header.num_blocks); - m_file.Read(Common::AsWritableU8Span(m_block_pointers)); + if (!m_file.Read(Common::AsWritableU8Span(m_block_pointers))) + return false; m_hashes.resize(m_header.num_blocks); - m_file.Read(Common::AsWritableU8Span(m_hashes)); + if (!m_file.Read(Common::AsWritableU8Span(m_hashes))) + return false; - m_data_offset = (sizeof(CompressedBlobHeader)) + - (sizeof(u64)) * m_header.num_blocks // skip block pointers - + (sizeof(u32)) * m_header.num_blocks; // skip hashes + m_data_offset = header_size; // A compressed block is never ever longer than a decompressed block, so just header.block_size // should be fine. // I still add some safety margin. const u32 zlib_buffer_size = m_header.block_size + 64; m_zlib_buffer.resize(zlib_buffer_size); + + SetSectorSize(m_header.block_size); + + return ValidateBlockPointers(); } std::unique_ptr CompressedBlobReader::Create(File::DirectIOFile file, const std::string& filename) { if (IsGCZBlob(file)) - return std::unique_ptr( + { + std::unique_ptr reader( new CompressedBlobReader(std::move(file), filename)); + if (reader->m_valid) + return reader; + } + return nullptr; } @@ -171,6 +211,46 @@ bool CompressedBlobReader::GetBlock(u64 block_num, u8* out_ptr) return true; } +bool CompressedBlobReader::ValidateBlockPointers() const +{ + size_t valid_pointers = 0; + + // Validate block pointers + for (u32 i = 0; i < m_header.num_blocks; ++i) + { + u64 next; + if (i + 1 < m_header.num_blocks) + next = m_block_pointers[i + 1] & ~uncompressed_flag; + else + next = m_header.compressed_data_size; + + if (next > m_header.compressed_data_size) + continue; + + u64 offset = m_block_pointers[i] & ~uncompressed_flag; + if (offset > m_header.compressed_data_size) + continue; + + bool uncompressed = m_block_pointers[i] & uncompressed_flag; + u64 size = next - offset; + + if (uncompressed && size != m_header.block_size) + continue; + + if (!uncompressed && size > m_zlib_buffer.size()) + continue; + + valid_pointers++; + } + + size_t invalid_pointers = m_header.num_blocks - valid_pointers; + + if (invalid_pointers > 0) + ERROR_LOG_FMT(DISCIO, "GCZ file has {} invalid block pointers", invalid_pointers); + + return invalid_pointers == 0; +} + struct CompressThreadState { CompressThreadState() : z{} {} @@ -258,7 +338,7 @@ static ConversionResultCode Output(OutputParameters parameters, File::DirectIOFi { u64 offset = *position; if (!parameters.compressed) - offset |= 0x8000000000000000ULL; + offset |= uncompressed_flag; (*offsets)[parameters.block_number] = offset; *position += parameters.data.size(); @@ -306,10 +386,10 @@ bool ConvertToGCZ(BlobReader* infile, const std::string& infile_path, header.magic_cookie = GCZ_MAGIC; header.sub_type = sub_type; header.block_size = block_size; - header.data_size = infile->GetDataSize(); + header.disc_size = infile->GetDataSize(); // round upwards! - header.num_blocks = (u32)((header.data_size + (block_size - 1)) / block_size); + header.num_blocks = (u32)((header.disc_size + (block_size - 1)) / block_size); std::vector offsets(header.num_blocks); std::vector hashes(header.num_blocks); @@ -345,7 +425,7 @@ bool ConvertToGCZ(BlobReader* infile, const std::string& infile_path, if (compressor.GetStatus() != ConversionResultCode::Success) break; - const u64 bytes_to_read = std::min(block_size, header.data_size - inpos); + const u64 bytes_to_read = std::min(block_size, header.disc_size - inpos); if (!infile->Read(inpos, bytes_to_read, in_buf.data())) { diff --git a/Source/Core/DiscIO/CompressedBlob.h b/Source/Core/DiscIO/CompressedBlob.h index 943353b169..6f44e77976 100644 --- a/Source/Core/DiscIO/CompressedBlob.h +++ b/Source/Core/DiscIO/CompressedBlob.h @@ -35,7 +35,7 @@ struct CompressedBlobHeader // 32 bytes u32 magic_cookie; // 0xB10BB10B u32 sub_type; // GC image, whatever u64 compressed_data_size; - u64 data_size; + u64 disc_size; u32 block_size; u32 num_blocks; }; @@ -53,7 +53,7 @@ public: std::unique_ptr CopyReader() const override; u64 GetRawSize() const override { return m_file_size; } - u64 GetDataSize() const override { return m_header.data_size; } + u64 GetDataSize() const override { return m_header.disc_size; } DataSizeType GetDataSizeType() const override { return DataSizeType::Accurate; } u64 GetBlockSize() const override { return m_header.block_size; } @@ -66,15 +66,18 @@ public: private: CompressedBlobReader(File::DirectIOFile file, std::string filename); - - CompressedBlobHeader m_header; - std::vector m_block_pointers; - std::vector m_hashes; - int m_data_offset; - File::DirectIOFile m_file; - u64 m_file_size; - std::vector m_zlib_buffer; - std::string m_file_name; + bool Initialize(); + bool ValidateBlockPointers() const; + + CompressedBlobHeader m_header = {}; + std::vector m_block_pointers = {}; + std::vector m_hashes = {}; + int m_data_offset = 0; + File::DirectIOFile m_file = {}; + u64 m_file_size = 0; + std::vector m_zlib_buffer = {}; + std::string m_file_name = {}; + bool m_valid = false; }; } // namespace DiscIO -- cgit v1.2.3 From 4782a04c2032e02ba7785b1378036ba30d579ce9 Mon Sep 17 00:00:00 2001 From: Scott Mansell Date: Sun, 26 Jul 2026 22:22:18 +1200 Subject: GCZ: use 64-bit for m_data_offset A malicious GCZ file could probably force this to be negative. Shouldn't cause any issues other than file read failures, but need to fix because it is causing errors on MSVC. --- Source/Core/DiscIO/CompressedBlob.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/Core/DiscIO/CompressedBlob.h b/Source/Core/DiscIO/CompressedBlob.h index 6f44e77976..712bb93917 100644 --- a/Source/Core/DiscIO/CompressedBlob.h +++ b/Source/Core/DiscIO/CompressedBlob.h @@ -72,7 +72,7 @@ private: CompressedBlobHeader m_header = {}; std::vector m_block_pointers = {}; std::vector m_hashes = {}; - int m_data_offset = 0; + u64 m_data_offset = 0; File::DirectIOFile m_file = {}; u64 m_file_size = 0; std::vector m_zlib_buffer = {}; -- cgit v1.2.3