fix: accept downloads by cryptographic hash only

A crc32 and a size can be met by crafted bytes, so a remote download
matched on them could land in bios/. verify_content now requires every
sha256, sha1 and md5 the entry declares to match, and an entry declaring
none is reported as not verifiable from the network and nothing is
written. crc32 and size still find a file the database already holds.
This commit is contained in:
Abdessamad Derraz committed 2026-10-21 10:04:20 +02:00
1 parent 6c076884ed
commit d16f8fd17a
3 files changed
+67 -34

No files matched your search

+28 -26
View File
@@ -25,7 +25,6 @@ import sys
import urllib.error
import urllib.parse
import urllib.request
import zlib
from pathlib import Path
sys.path.insert(0, os.path.dirname(os.path.abspath(__file__)))
@@ -147,35 +146,38 @@ def _size(entry: dict) -> int | None:
return None
def verifiable(entry: dict) -> bool:
"""Whether a download can be proved to be this file.
CRYPTO_HASHES = (
("sha256", hashlib.sha256),
("sha1", hashlib.sha1),
("md5", hashlib.md5),
)
A sha1, sha256 or md5 proves it, a crc32 only with the size beside it,
the way the resolver accepts one. Without any of them a search can find
a file of that name and never accept it.
def verifiable(entry: dict) -> bool:
"""Whether a network download can be proved to be this file.
Only a cryptographic hash proves it. A crc32 and a size can be met by
crafted bytes, so they identify a file the database already holds
(step 1) and never accept one a remote source serves.
"""
if any(_accepted(entry, field) for field in ("sha1", "sha256", "md5")):
return True
return bool(_crc32s(entry)) and _size(entry) is not None
return any(_accepted(entry, field) for field, _digest in CRYPTO_HASHES)
def verify_content(data: bytes, expected: dict) -> bool:
"""Verify downloaded content matches one of the expected hashes.
"""Whether downloaded bytes match every cryptographic hash declared.
The strongest hash the entry declares decides.
Each declared field must hold the bytes' digest among its accepted
values. An entry declaring none is refused.
"""
for field, digest in (
("sha1", hashlib.sha1),
("sha256", hashlib.sha256),
("md5", hashlib.md5),
):
accepted = _accepted(expected, field)
if accepted:
return digest(data).hexdigest() in accepted
crcs, size = _crc32s(expected), _size(expected)
if crcs and size is not None:
return len(data) == size and f"{zlib.crc32(data) & 0xFFFFFFFF:08x}" in crcs
return False
declared = [
(accepted, digest)
for field, digest in CRYPTO_HASHES
for accepted in [_accepted(expected, field)]
if accepted
]
return bool(declared) and all(
digest(data).hexdigest() in accepted for accepted, digest in declared
)
def _read_limited(
@@ -413,8 +415,8 @@ def fetch_missing(
if not verifiable(entry):
# Searched, a file of that name could only ever be refused.
print(" [-] No sha1, sha256, md5 or crc32 with size to check a "
"download against: not searched")
print(" [-] Cannot be verified from the network: declares no "
"cryptographic hash. Not searched, nothing written")
still_missing.append(entry)
stats["unverifiable"] += 1
continue
@@ -565,7 +567,7 @@ def main() -> None:
print(
f"\n Results: {stats['found']} found, {stats['not_found']} not found, "
f"{stats['errors']} not fully searched, "
f"{stats['unverifiable']} without a hash to search by"
f"{stats['unverifiable']} without a cryptographic hash"
)
if args.create_issues and all_still_missing:
+38 -7
View File
@@ -5605,19 +5605,50 @@ class AutoFetchProvesEveryHashItIsGiven(unittest.TestCase):
DATA = b"firmware bytes"
def test_sha256_and_crc32_with_size_prove_a_download(self):
def test_a_sha256_proves_a_download(self):
import hashlib
import zlib
from scripts import auto_fetch
sha256 = {"sha256": hashlib.sha256(self.DATA).hexdigest()}
self.assertTrue(auto_fetch.verify_content(self.DATA, sha256))
self.assertFalse(auto_fetch.verify_content(b"other", sha256))
crc = {"crc32": f"{zlib.crc32(self.DATA):08X}", "size": len(self.DATA)}
self.assertTrue(auto_fetch.verify_content(self.DATA, crc))
self.assertFalse(auto_fetch.verify_content(self.DATA, dict(crc, size=1)))
self.assertFalse(auto_fetch.verifiable({"crc32": crc["crc32"]}))
def test_every_declared_cryptographic_hash_must_match(self):
import hashlib
from scripts import auto_fetch
both = {"sha256": hashlib.sha256(self.DATA).hexdigest(), "md5": "0" * 32}
self.assertFalse(auto_fetch.verify_content(self.DATA, both))
both["md5"] = hashlib.md5(self.DATA).hexdigest()
self.assertTrue(auto_fetch.verify_content(self.DATA, both))
def test_crc32_and_size_never_accept_a_download(self):
"""A crc32 can be forged: bytes a source crafts to it would land in
bios/."""
import urllib.request
import zlib
from unittest import mock
from scripts import auto_fetch
crc = {"name": "forged.bin", "system": "test-system",
"crc32": f"{zlib.crc32(self.DATA):08X}", "size": len(self.DATA)}
self.assertFalse(auto_fetch.verify_content(self.DATA, crc))
self.assertFalse(auto_fetch.verifiable(crc))
served = io.BytesIO(self.DATA)
with tempfile.TemporaryDirectory() as bios, \
mock.patch.object(urllib.request, "urlopen", lambda *a, **k: served), \
mock.patch.object(auto_fetch, "step2_scan_branches",
lambda e: self.DATA), \
contextlib.redirect_stdout(io.StringIO()) as out:
result = auto_fetch.fetch_missing([crc], {"files": {}}, bios)
self.assertEqual(list(Path(bios).rglob("*")), [])
self.assertEqual(result["stats"]["found"], 0)
self.assertEqual(result["stats"]["unverifiable"], 1)
self.assertIn("declares no cryptographic hash", out.getvalue())
def test_a_held_sha256_is_found_in_the_database(self):
from scripts import auto_fetch
@@ -5641,7 +5672,7 @@ class AutoFetchProvesEveryHashItIsGiven(unittest.TestCase):
branches.assert_not_called()
self.assertEqual(result["stats"]["unverifiable"], 1)
self.assertNotIn("Would search", out.getvalue())
self.assertIn("not searched", out.getvalue())
self.assertIn("Not searched", out.getvalue())
class AForcedRehashIsCheckedAgainstTheLastOne(unittest.TestCase):
+1 -1
View File
@@ -653,7 +653,7 @@ python scripts/refresh_stale.py --only platforms,targets --jobs 6
| `region_audit.py` | Confront every profile `region:` with the regions No-Intro names for the same dump (`--json`, `--db`, `--emulators-dir`). An opinion, never an authority: the code decides, and a disagreement is a line to read in the emulator's source |
| `validate_pr.py` | Validate BIOS files in pull requests, post markdown report |
| `validate_schemas.py` | Validate the data contracts: schemas and semantic invariants. `--source-only` checks `emulators/` and `platforms/` alone, which is what PR validation runs |
| `auto_fetch.py` | Fetch missing BIOS files from known sources (4-step pipeline). A download is accepted by the sha1, sha256 or md5 the entry declares, or its crc32 with the size. An entry with none of them is reported and not searched |
| `auto_fetch.py` | Fetch missing BIOS files from known sources (4-step pipeline). A download is accepted only when every cryptographic hash the entry declares (sha256, sha1, md5) matches. crc32 and size find a file the database already holds, never accept a download. An entry with no cryptographic hash is reported and not searched |
| `list_platforms.py` | List active platforms (`--include-archived` adds the archived ones, used by CI) |
| `download.py` | Download a pack from GitHub releases, every part checked (Python, stdlib only) |
| `download.sh` | Same, as a shell one-liner (`curl` + `unzip`) |