mirror of
https://github.com/Abdess/retroarch_system.git
synced 2026-10-20 18:59:07 -05:00
fix: the pack check applies the frontend's rule
The integrity step compared a member's md5 by hand and ignored hash_rule, so under RomM's rule (size, then any of md5, sha1 and crc32) it would refuse a revision the builder rightly shipped on its sha1. The rule now lives in digest_verdict, which the path check and the pack member check both call. The member's archive digest is computed without the path cache, whose key a buffer's reused address made collide.
This commit is contained in:
1 parent
20b4ce37e6
commit
ac47faf7f2
4 files changed
+111
-40
No files matched your search
+14
-4
@@ -10,6 +10,7 @@ import os
|
||||
import zipfile
|
||||
import zlib
|
||||
from pathlib import Path
|
||||
from typing import BinaryIO
|
||||
|
||||
|
||||
_ALL_ALGORITHMS = frozenset({"sha1", "md5", "sha256", "crc32", "adler32"})
|
||||
@@ -102,7 +103,18 @@ def md5_composite(filepath: str | Path) -> str:
|
||||
cached = _md5_composite_cache.get(key)
|
||||
if cached is not None:
|
||||
return cached
|
||||
with zipfile.ZipFile(filepath) as zf:
|
||||
result = md5_composite_of(filepath)
|
||||
_md5_composite_cache[key] = result
|
||||
return result
|
||||
|
||||
|
||||
def md5_composite_of(source: str | Path | BinaryIO) -> str:
|
||||
"""md5_composite without the cache, for an archive held in memory.
|
||||
|
||||
A buffer has no stable name: keyed by its repr, a later buffer at the
|
||||
same address read back another archive's digest.
|
||||
"""
|
||||
with zipfile.ZipFile(source) as zf:
|
||||
names = sorted(n for n in zf.namelist() if not n.endswith("/"))
|
||||
h = hashlib.md5()
|
||||
for name in names:
|
||||
@@ -110,9 +122,7 @@ def md5_composite(filepath: str | Path) -> str:
|
||||
if info.file_size > 512 * 1024 * 1024:
|
||||
continue # skip oversized entries
|
||||
h.update(zf.read(name))
|
||||
result = h.hexdigest()
|
||||
_md5_composite_cache[key] = result
|
||||
return result
|
||||
return h.hexdigest()
|
||||
|
||||
def parse_hash_list(raw: str | list | int | None) -> list[str]:
|
||||
"""Normalize any hash field into its accepted values, lowercase.
|
||||
|
||||
+30
-23
@@ -17,12 +17,14 @@ from packpaths import _register_path
|
||||
from ziptools import build_zip_contents_index
|
||||
from ziptools import check_inside_zip
|
||||
from nativemode import digest_algorithm
|
||||
from validation import baseline_verdict
|
||||
from validation import baseline_verdict, digest_verdict
|
||||
from nativemode import hash_mismatch_excludes_file
|
||||
import functools
|
||||
import hashlib
|
||||
import zlib
|
||||
import sys
|
||||
from common import filter_systems_by_target
|
||||
from hashing import parse_md5_list
|
||||
from hashing import md5_composite_of, parse_md5_list
|
||||
import io
|
||||
from common import load_emulator_profiles
|
||||
from common import load_platform_config
|
||||
@@ -328,7 +330,7 @@ def _hash_matches(declared: str, actual: str) -> bool:
|
||||
return False
|
||||
|
||||
def _check_member_hash(
|
||||
zf: zipfile.ZipFile, member: str, file_entry: dict, mode: str
|
||||
zf: zipfile.ZipFile, member: str, file_entry: dict, mode: str, rule: str = ""
|
||||
) -> str | None:
|
||||
"""Verify a pack member against its platform-declared hash.
|
||||
|
||||
@@ -363,31 +365,35 @@ def _check_member_hash(
|
||||
)
|
||||
return None
|
||||
|
||||
h = hashlib.md5() if mode == "md5" else hashlib.sha1()
|
||||
# The frontend's own rule, as the builder and verify apply it: RomM's
|
||||
# size-then-any-hash included, which a bare md5 comparison refused.
|
||||
digests = {"md5": hashlib.md5(), "sha1": hashlib.sha1()}
|
||||
crc = 0
|
||||
with zf.open(member) as f:
|
||||
for chunk in iter(lambda: f.read(65536), b""):
|
||||
h.update(chunk)
|
||||
actual = h.hexdigest()
|
||||
if _hash_matches(declared, actual):
|
||||
return None
|
||||
for h in digests.values():
|
||||
h.update(chunk)
|
||||
crc = zlib.crc32(chunk, crc)
|
||||
actual = {name: h.hexdigest() for name, h in digests.items()}
|
||||
actual["crc32"] = format(crc & 0xFFFFFFFF, "08x")
|
||||
|
||||
# Recalbox Md5Composite: MD5 over sorted inner contents of a ZIP,
|
||||
# independent of compression and metadata (Zip::Md5Composite()).
|
||||
if mode == "md5" and member.endswith(".zip"):
|
||||
def composite() -> str | None:
|
||||
# Recalbox's Md5Composite, over the sorted members of a ZIP.
|
||||
if not member.endswith(".zip"):
|
||||
return None
|
||||
import io
|
||||
|
||||
try:
|
||||
with zipfile.ZipFile(io.BytesIO(zf.read(member))) as inner:
|
||||
names = sorted(n for n in inner.namelist() if not n.endswith("/"))
|
||||
ch = hashlib.md5()
|
||||
for n in names:
|
||||
ch.update(inner.read(n))
|
||||
if _hash_matches(declared, ch.hexdigest()):
|
||||
return None
|
||||
except (zipfile.BadZipFile, OSError):
|
||||
pass
|
||||
return md5_composite_of(io.BytesIO(zf.read(member)))
|
||||
except (zipfile.BadZipFile, OSError) as exc:
|
||||
print(f" {member}: no member digest ({exc})", file=sys.stderr)
|
||||
return None
|
||||
|
||||
return f"{member}: {mode} {actual} != declared {declared}"
|
||||
if digest_verdict(
|
||||
file_entry, zf.getinfo(member).file_size, lambda: actual, mode, rule, composite
|
||||
):
|
||||
return None
|
||||
return f"{member}: {mode} {actual[mode]} != declared {declared}"
|
||||
|
||||
def _repo_satisfies_declaration(
|
||||
entries: list[dict], db: dict, mode: str
|
||||
@@ -666,6 +672,7 @@ def verify_pack_against_platform(
|
||||
# one, the pack ships its best effort and the gap is a data issue
|
||||
# reported by verify.py, not a generation bug.
|
||||
digest = digest_algorithm(verification_mode)
|
||||
hash_rule = config.get("hash_rule", "")
|
||||
if reads_file_contents(verification_mode) and digest:
|
||||
for member, decl_entries in decl_by_member.items():
|
||||
checkable = [
|
||||
@@ -682,13 +689,13 @@ def verify_pack_against_platform(
|
||||
for fe in checkable:
|
||||
if not fe.get("zipped_file"):
|
||||
continue
|
||||
err = _check_member_hash(zf, member, fe, digest)
|
||||
err = _check_member_hash(zf, member, fe, digest, hash_rule)
|
||||
if err is not None and (db is None or _repo_holds_member(fe, db)):
|
||||
errors.append(err)
|
||||
member_errors = []
|
||||
satisfied = False
|
||||
for fe in checkable:
|
||||
err = _check_member_hash(zf, member, fe, digest)
|
||||
err = _check_member_hash(zf, member, fe, digest, hash_rule)
|
||||
if err is None:
|
||||
satisfied = True
|
||||
break
|
||||
|
||||
+39
-13
@@ -9,7 +9,7 @@ from __future__ import annotations
|
||||
|
||||
import os
|
||||
import zipfile
|
||||
from collections.abc import Iterator, Mapping
|
||||
from collections.abc import Callable, Iterator, Mapping
|
||||
|
||||
from common import (
|
||||
adler32_of,
|
||||
@@ -652,13 +652,43 @@ def frontend_digest_matches(
|
||||
match and then any one of md5, sha1 and crc32 does: a mistyped hash next
|
||||
to a right one does not withhold the file, a wrong size does.
|
||||
"""
|
||||
if not local_path:
|
||||
return False
|
||||
|
||||
def composite() -> str | None:
|
||||
if ".zip" not in os.path.basename(local_path):
|
||||
return None
|
||||
try:
|
||||
return md5_composite(local_path)
|
||||
except (zipfile.BadZipFile, OSError):
|
||||
return None
|
||||
|
||||
return digest_verdict(
|
||||
file_entry, os.path.getsize(local_path), lambda: compute_hashes(local_path),
|
||||
algorithm, rule, composite,
|
||||
)
|
||||
|
||||
|
||||
def digest_verdict(
|
||||
file_entry: dict,
|
||||
size: int,
|
||||
hashes: Callable[[], dict[str, str]],
|
||||
algorithm: str,
|
||||
rule: str = "",
|
||||
composite: Callable[[], str | None] = lambda: None,
|
||||
) -> bool:
|
||||
"""The frontend's acceptance rule over content given by its size, its
|
||||
digests and, for an archive, its member digest.
|
||||
|
||||
One rule for a file on disk and for a member of a built pack: the pack
|
||||
check once compared only the md5 and refused, under RomM's rule, the
|
||||
revision the builder had rightly shipped on its sha1 and size.
|
||||
"""
|
||||
if rule == SIZE_AND_ANY_HASH:
|
||||
if not local_path:
|
||||
return False
|
||||
# RomM reads an absent size as 0: such an entry never verifies.
|
||||
if os.path.getsize(local_path) != int(file_entry.get("size") or 0):
|
||||
if size != int(file_entry.get("size") or 0):
|
||||
return False
|
||||
actual = compute_hashes(local_path)
|
||||
actual = hashes()
|
||||
return any(
|
||||
actual[field].lower() == str(file_entry[field]).strip().lower()
|
||||
for field in ("md5", "sha1", "crc32")
|
||||
@@ -670,19 +700,15 @@ def frontend_digest_matches(
|
||||
value = file_entry.get(algorithm)
|
||||
values = value if isinstance(value, list) else [value]
|
||||
declared = [str(v).strip().lower() for v in values if v]
|
||||
if not declared or not local_path:
|
||||
if not declared:
|
||||
return False
|
||||
actual = compute_hashes(local_path)[algorithm].lower()
|
||||
actual = hashes()[algorithm].lower()
|
||||
if algorithm != "md5":
|
||||
return actual in declared
|
||||
if any(actual == d or (len(d) < 32 and actual.startswith(d)) for d in declared):
|
||||
return True
|
||||
if ".zip" not in os.path.basename(local_path):
|
||||
return False
|
||||
try:
|
||||
return md5_composite(local_path).lower() in declared
|
||||
except (zipfile.BadZipFile, OSError):
|
||||
return False
|
||||
member_digest = composite()
|
||||
return bool(member_digest) and member_digest.lower() in declared
|
||||
|
||||
|
||||
def inner_rom_check(file_entry: dict, local_path: str) -> str:
|
||||
|
||||
@@ -5327,5 +5327,33 @@ class TheCliSetsBundledFilesApart(unittest.TestCase):
|
||||
self.assertIn("n64.psh [BUNDLED]", text)
|
||||
|
||||
|
||||
class ThePackCheckAppliesTheFrontendRule(unittest.TestCase):
|
||||
"""The integrity step compared a member's md5 alone. Under RomM's rule
|
||||
(size, then any of md5, sha1 and crc32) the builder ships a revision
|
||||
matched on its sha1, and the check then refused a correct pack."""
|
||||
|
||||
def test_romm_accepts_on_size_and_sha1(self):
|
||||
import hashlib
|
||||
import zipfile
|
||||
|
||||
from scripts.packverify import _check_member_hash
|
||||
|
||||
payload = b"revision B"
|
||||
entry = {
|
||||
"md5": "0" * 32,
|
||||
"sha1": hashlib.sha1(payload).hexdigest(),
|
||||
"size": len(payload),
|
||||
}
|
||||
with tempfile.TemporaryDirectory(dir=TMP_ROOT) as tmp:
|
||||
pack = Path(tmp) / "p.zip"
|
||||
with zipfile.ZipFile(pack, "w") as zf:
|
||||
zf.writestr("bios/x.bin", payload)
|
||||
with zipfile.ZipFile(pack) as zf:
|
||||
self.assertIsNone(
|
||||
_check_member_hash(zf, "bios/x.bin", entry, "md5", "size_and_any_hash")
|
||||
)
|
||||
self.assertIsNotNone(_check_member_hash(zf, "bios/x.bin", entry, "md5"))
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
Reference in new issue
Block a user