Refactor/fix verify subcommand

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

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)

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 \

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 \

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;

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<std::string> &listfile_name, b
                                   << FileTimeToLsTime(GetFileInfo<int64_t>(file, it->second))
                                   << " ";
                     } else if (prop == "file-size" || prop == "compressed-size") {
-                        std::cout << std::setw(8) << GetFileInfo<int32_t>(file, it->second) << " ";
+                        std::cout << std::setw(8) << GetFileInfo<uint32_t>(file, it->second) << " ";
                     } else if (prop == "flags") {
                         std::cout << std::setw(8)
                                   << GetFlagString(GetFileInfo<int32_t>(file, it->second)) << " ";
@@ -601,7 +601,7 @@ void PrintMpqInfo(HANDLE archive, const std::optional<std::string> &info_propert
          }},
         {"archive-size",
          [&](bool print_name) {
-             int32_t archive_size = GetFileInfo<int32_t>(archive, SFileMpqArchiveSize);
+             int64_t archive_size = GetFileInfo<int64_t>(archive, SFileMpqArchiveSize64);
              if (print_name) {
                  std::cout << "Archive size: ";
              }
@@ -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<std::vector<char>>(archive, SFileMpqStrongSignature);
-        if (signature_content.empty()) {
-            int64_t archive_size = GetFileInfo<int64_t>(archive, SFileMpqArchiveSize64);
-            int64_t archive_offset = GetFileInfo<int64_t>(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<char>)), so read it directly from the archive file instead.
+        int64_t archive_size = GetFileInfo<int64_t>(archive, SFileMpqArchiveSize64);
+        int64_t archive_offset = GetFileInfo<int64_t>(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<size_t>(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<size_t>(signature_length));
+        file_mpq.read(signature_content.data(), signature_content.size());
+        file_mpq.close();
 
-            PrintAsBinary(signature_content.data(),
-                          static_cast<uint32_t>(signature_content.size()));
-        }
+        PrintAsBinary(signature_content.data(), static_cast<uint32_t>(signature_content.size()));
     }
 
     return 0;

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

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):