From 544c35b152cdf2bc5c6603e25e703e29be370bba Mon Sep 17 00:00:00 2001 From: thomaslaurenson Date: Fri, 3 Jul 2026 12:01:15 +1200 Subject: [PATCH 1/5] Fixes for verify subcommand --- src/commands.cpp | 34 +++++++++++++++++------ test/conftest.py | 68 +++++++++++++++++++++++++++++++++++++++++++++ test/test_verify.py | 41 +++++++++++++++++++++++++-- 3 files changed, 131 insertions(+), 12 deletions(-) diff --git a/src/commands.cpp b/src/commands.cpp index 804b08a..14e7de0 100644 --- a/src/commands.cpp +++ b/src/commands.cpp @@ -328,24 +328,40 @@ int HandleVerify(const std::string &target, bool print_signature) { return 1; } - int result = 0; - uint32_t verify_result = VerifyMpqArchive(archive); - if (verify_result == ERROR_WEAK_SIGNATURE_OK || verify_result == ERROR_STRONG_SIGNATURE_OK || - verify_result == ERROR_WEAK_SIGNATURE_ERROR || - verify_result == ERROR_STRONG_SIGNATURE_ERROR) { + int result; + switch (VerifyMpqArchive(archive)) { + case ERROR_WEAK_SIGNATURE_OK: + case ERROR_STRONG_SIGNATURE_OK: if (print_signature) { // If printing the signature, don't print success message // because the user might want to pipe/redirect the signature data PrintMpqSignature(archive, target); } else { - // Just print verification success std::cout << "[*] Verify success" << std::endl; } result = 0; - } else { - // Any other verify result is no signature, or error verifying - std::cout << "[!] Verify failed" << std::endl; + break; + + case ERROR_WEAK_SIGNATURE_ERROR: + case ERROR_STRONG_SIGNATURE_ERROR: + if (print_signature) { + // Print the (invalid) signature bytes for forensic inspection, + // but still fail: the archive content no longer matches it + PrintMpqSignature(archive, target); + } + std::cerr << "[!] Verify failed: signature is present but invalid" << std::endl; + result = 1; + break; + + case ERROR_NO_SIGNATURE: + std::cerr << "[!] Verify failed: archive has no signature" << std::endl; + result = 1; + break; + + default: // ERROR_VERIFY_FAILED or any other value + std::cerr << "[!] Verify failed" << std::endl; result = 1; + break; } CloseMpqArchive(archive); return result; diff --git a/test/conftest.py b/test/conftest.py index f8bf64f..ed62f8e 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -208,6 +208,74 @@ def generate_path_traversal_mpq(binary_path): yield mpq_file +@pytest.fixture(scope="function") +def generate_tampered_signature_mpq(binary_path): + """ + Build a signed MPQ archive, then flip a single byte inside the file's + compressed data (well past the leading sector-offset-table bytes, which + are sensitive to corruption in ways unrelated to the signature). + + This produces an archive whose "(signature)" entry is present but no + longer matches the archive content, i.e. StormLib's + ERROR_WEAK_SIGNATURE_ERROR case. + """ + script_dir = Path(__file__).parent + + data_dir = script_dir / "data" + data_dir.mkdir(parents=True, exist_ok=True) + + tamper_files_dir = data_dir / "tamper_files" + shutil.rmtree(tamper_files_dir, ignore_errors=True) + tamper_files_dir.mkdir(parents=True, exist_ok=True) + + mpq_file = data_dir / "mpq_with_tampered_signature.mpq" + mpq_file.unlink(missing_ok=True) + + text_file = tamper_files_dir / "cats.txt" + text_file.write_text( + "This is a longer file about cats, with enough content to give us " + "several bytes of compressed data to safely flip a bit in.\n", + newline="\n", + ) + + result = subprocess.run( + [str(binary_path), "create", "-s", "-o", str(mpq_file), str(tamper_files_dir)], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True + ) + assert result.returncode == 0, f"mpqcli failed with error: {result.stderr}" + + offset_result = subprocess.run( + [str(binary_path), "list", "-p", "byte-offset", str(mpq_file)], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True + ) + assert offset_result.returncode == 0, f"mpqcli failed with error: {offset_result.stderr}" + byte_offset = int(offset_result.stdout.split()[0]) + + size_result = subprocess.run( + [str(binary_path), "list", "-p", "compressed-size", str(mpq_file)], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True + ) + assert size_result.returncode == 0, f"mpqcli failed with error: {size_result.stderr}" + compressed_size = int(size_result.stdout.split()[0]) + + # Flip a byte at the midpoint of the compressed data, away from the + # sector-offset-table bytes at the start of the blob + flip_offset = byte_offset + compressed_size // 2 + with open(mpq_file, "r+b") as f: + f.seek(flip_offset) + original_byte = f.read(1) + f.seek(flip_offset) + f.write(bytes([original_byte[0] ^ 0xFF])) + + yield mpq_file + + @pytest.fixture(scope="session") def download_test_files(): script_dir = Path(__file__).parent diff --git a/test/test_verify.py b/test/test_verify.py index 1196f93..23e3e1e 100644 --- a/test/test_verify.py +++ b/test/test_verify.py @@ -9,12 +9,13 @@ def test_verify_no_signature(binary_path): This test checks: - If the application exits correctly when the MPQ file has no signature. + - If the failure message distinguishes "no signature" from other failures. """ script_dir = Path(__file__).parent test_file = script_dir / "data" / "mpq_with_output_v1.mpq" expected_output = { - "[!] Verify failed", + "[!] Verify failed: archive has no signature", } result = subprocess.run( @@ -24,10 +25,44 @@ def test_verify_no_signature(binary_path): text=True ) - output_lines = set(result.stdout.splitlines()) + stdout_lines = set(result.stdout.splitlines()) + stderr_lines = set(result.stderr.splitlines()) assert result.returncode == 1, f"mpqcli failed with error: {result.stderr}" - assert output_lines == expected_output, f"Unexpected output: {output_lines}" + assert stdout_lines == set(), f"Unexpected stdout: {stdout_lines}" + assert stderr_lines == expected_output, f"Unexpected stderr: {stderr_lines}" + + +def test_verify_tampered_signature(binary_path, generate_tampered_signature_mpq): + """ + Test MPQ file verification with a signature that is present but invalid. + + This test checks: + - If the application detects an archive that was modified after signing + (signature present, but no longer valid for the current content). + - If the application exits with a non-zero code. + - If the failure message distinguishes "invalid signature" from the + "no signature" case, instead of printing the same generic failure. + """ + test_file = generate_tampered_signature_mpq + + expected_output = { + "[!] Verify failed: signature is present but invalid", + } + + result = subprocess.run( + [str(binary_path), "verify", str(test_file)], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True + ) + + stdout_lines = set(result.stdout.splitlines()) + stderr_lines = set(result.stderr.splitlines()) + + assert result.returncode == 1, f"mpqcli unexpectedly succeeded: {result.stdout}" + assert stdout_lines == set(), f"Unexpected stdout: {stdout_lines}" + assert stderr_lines == expected_output, f"Unexpected stderr: {stderr_lines}" def test_verify_weak_signature(binary_path): From 59f12cf7d488490e04152ee950006ea83fe782e4 Mon Sep 17 00:00:00 2001 From: thomaslaurenson Date: Fri, 3 Jul 2026 12:55:26 +1200 Subject: [PATCH 2/5] Fixed issue with overwrite flag and archive size reporting --- src/mpq.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/mpq.cpp b/src/mpq.cpp index 6e99232..c122881 100644 --- a/src/mpq.cpp +++ b/src/mpq.cpp @@ -325,7 +325,7 @@ int AddFile(HANDLE archive, const fs::path &local_file, const std::string &archi DWORD compression_next = overrides.compression_next.value_or(settings.compression_next); if (overwrite) { - flags += MPQ_FILE_REPLACEEXISTING; + flags |= MPQ_FILE_REPLACEEXISTING; } bool added_file = @@ -508,7 +508,7 @@ int ListFiles(HANDLE archive, const std::optional &listfile_name, b << FileTimeToLsTime(GetFileInfo(file, it->second)) << " "; } else if (prop == "file-size" || prop == "compressed-size") { - std::cout << std::setw(8) << GetFileInfo(file, it->second) << " "; + std::cout << std::setw(8) << GetFileInfo(file, it->second) << " "; } else if (prop == "flags") { std::cout << std::setw(8) << GetFlagString(GetFileInfo(file, it->second)) << " "; @@ -601,7 +601,7 @@ void PrintMpqInfo(HANDLE archive, const std::optional &info_propert }}, {"archive-size", [&](bool print_name) { - int32_t archive_size = GetFileInfo(archive, SFileMpqArchiveSize); + int64_t archive_size = GetFileInfo(archive, SFileMpqArchiveSize64); if (print_name) { std::cout << "Archive size: "; } From 5d40c313dbc939622f699c88ea8d1af01655f27e Mon Sep 17 00:00:00 2001 From: thomaslaurenson Date: Fri, 3 Jul 2026 13:06:27 +1200 Subject: [PATCH 3/5] Simplified how strong archive signatures are read internally --- src/mpq.cpp | 41 ++++++++++++++++++++--------------------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/src/mpq.cpp b/src/mpq.cpp index c122881..93b9cf2 100644 --- a/src/mpq.cpp +++ b/src/mpq.cpp @@ -692,29 +692,28 @@ int32_t PrintMpqSignature(HANDLE archive, const std::string &target) { PrintAsBinary(file_content.get(), file_size); } else if (signature_type == SIGNATURE_TYPE_STRONG) { - signature_content = GetFileInfo>(archive, SFileMpqStrongSignature); - if (signature_content.empty()) { - int64_t archive_size = GetFileInfo(archive, SFileMpqArchiveSize64); - int64_t archive_offset = GetFileInfo(archive, SFileMpqHeaderOffset); - - const fs::path archive_path = fs::canonical(target); - std::uintmax_t file_size = fs::file_size(archive_path); - int64_t signature_length = file_size - archive_offset - archive_size; - - if (signature_length <= 0) { - std::cerr << "[!] Invalid signature length: " << signature_length << std::endl; - return -1; - } + // StormLib does not expose the strong signature via SFileGetFileInfo into a + // growable buffer (SFileMpqStrongSignature's advertised size always exceeds + // sizeof(std::vector)), so read it directly from the archive file instead. + int64_t archive_size = GetFileInfo(archive, SFileMpqArchiveSize64); + int64_t archive_offset = GetFileInfo(archive, SFileMpqHeaderOffset); + + const fs::path archive_path = fs::canonical(target); + std::uintmax_t file_size = fs::file_size(archive_path); + int64_t signature_length = file_size - archive_offset - archive_size; + + if (signature_length <= 0) { + std::cerr << "[!] Invalid signature length: " << signature_length << std::endl; + return -1; + } - std::ifstream file_mpq(archive_path, std::ios::binary); - file_mpq.seekg(archive_offset + archive_size, std::ios::beg); - signature_content.resize(static_cast(signature_length)); - file_mpq.read(signature_content.data(), signature_content.size()); - file_mpq.close(); + std::ifstream file_mpq(archive_path, std::ios::binary); + file_mpq.seekg(archive_offset + archive_size, std::ios::beg); + signature_content.resize(static_cast(signature_length)); + file_mpq.read(signature_content.data(), signature_content.size()); + file_mpq.close(); - PrintAsBinary(signature_content.data(), - static_cast(signature_content.size())); - } + PrintAsBinary(signature_content.data(), static_cast(signature_content.size())); } return 0; From f1a7515637f468788334069a77d10146694a6c37 Mon Sep 17 00:00:00 2001 From: thomaslaurenson Date: Fri, 3 Jul 2026 13:10:16 +1200 Subject: [PATCH 4/5] Typo is dockerfile label --- Dockerfile.glibc | 2 +- Dockerfile.musl | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Dockerfile.glibc b/Dockerfile.glibc index 6a06e39..8310658 100644 --- a/Dockerfile.glibc +++ b/Dockerfile.glibc @@ -1,4 +1,4 @@ -# Stage 1: Build and Test +# Stage 1: Build FROM ubuntu:24.04 AS builder RUN apt-get update && apt-get install -y \ diff --git a/Dockerfile.musl b/Dockerfile.musl index 5c1f21e..012213d 100644 --- a/Dockerfile.musl +++ b/Dockerfile.musl @@ -1,4 +1,4 @@ -# Stage 1: Build and Test +# Stage 1: Build FROM alpine:3.23 AS builder RUN apk add --no-cache \ From b9fd145b582af045034bacf70e808cada752b239 Mon Sep 17 00:00:00 2001 From: thomaslaurenson Date: Fri, 3 Jul 2026 13:10:56 +1200 Subject: [PATCH 5/5] Prep for version bump --- CHANGELOG.md | 14 ++++++++++++++ CMakeLists.txt | 2 +- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c8beba5..9fdcd90 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,19 @@ # Changelog +## 0.10.2 - 2026-07-03 + +### Fixed + +- `verify` incorrectly reporting success for a tampered archive +- `verify` failure messages now clarify whether a signature is missing or invalid +- `add` overwrite flag not always working as expected +- Archive size incorrectly reported for large archives +- File sizes shown as negative for large files + +### Updated + +- Simplified how strong archive signatures are read internally + ## 0.10.1 - 2026-07-03 ### Added diff --git a/CMakeLists.txt b/CMakeLists.txt index be7dcdf..71dd805 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,6 +1,6 @@ cmake_minimum_required(VERSION 3.10) -project(MPQCLI VERSION 0.10.1) +project(MPQCLI VERSION 0.10.2) # Options option(BUILD_MPQCLI "Build the mpqcli CLI app" ON)