From 35e9e890b59b28f094e4b4b09b00e37371f7331d Mon Sep 17 00:00:00 2001 From: Eric Lewis Date: Sun, 10 May 2026 00:29:49 -0400 Subject: [PATCH] Validate firmware image before patching Why: The patcher trusted firmware files even when the header did not match the file body. That could create a broken patched firmware file that still looked valid. This checks the header, loader files, and Rev B patch location before writing output, so bad inputs stop early. --- combine.py | 106 ++++++++++++++++++++++++++++++------------ tests/test_combine.py | 93 ++++++++++++++++++++++++++++++++++++ 2 files changed, 169 insertions(+), 30 deletions(-) create mode 100644 tests/test_combine.py diff --git a/combine.py b/combine.py index a5be11f..f3198d7 100644 --- a/combine.py +++ b/combine.py @@ -3,33 +3,72 @@ from firmware_addresses import DEFAULT_VERSION, FirmwareAddressError, get_firmware_addresses -print("Enter target system: A for Rev. A, B for Rev. B, or AB for both") -target = input("|)> ") +FIRMWARE_HEADER_SIZE = 32 +LENGTH_OFFSET = 8 +HASH_OFFSET = 24 +HASH_SIZE = 8 -#TODO: add ptr patching to REV. A once i figure out how to develop cfw for it +def read_firmware_image(path): + with open(path, "rb") as f: + fw_header = bytearray(f.read(FIRMWARE_HEADER_SIZE)) + fw = f.read() + if len(fw_header) != FIRMWARE_HEADER_SIZE: + raise ValueError(f"{path}: expected 32-byte header, got {len(fw_header)} bytes") -if "A" in target.upper(): - with open("pdfw-a", "rb") as f: - fw_header = bytearray(f.read(32)) - fw = f.read() - + expected_length = int.from_bytes(fw_header[LENGTH_OFFSET:LENGTH_OFFSET + 4], byteorder="little") + if expected_length != len(fw): + raise ValueError(f"{path}: header body length {expected_length} does not match actual body length {len(fw)}") + + expected_hash = bytes(fw_header[HASH_OFFSET:HASH_OFFSET + HASH_SIZE]) + actual_hash = md5(fw).digest()[:HASH_SIZE] + if expected_hash != actual_hash: + raise ValueError(f"{path}: header body hash does not match actual body hash") + + return fw_header, fw + + +def read_loader_binary(path): + try: + with open(path, "rb") as f: + return f.read() + except FileNotFoundError: + raise FileNotFoundError(f"required loader input not found: {path}") from None + except OSError as exc: + raise OSError(f"required loader input is not readable: {path}") from exc + + +def patch_rev_b_pointer(fw, ptr_addr, update_func_addr): + if ptr_addr < 0 or ptr_addr + 4 > len(fw): + raise ValueError(f"Rev. B patch address 0x{ptr_addr:x} is outside firmware body length {len(fw)}") + struct.pack_into(" ") + + #TODO: add ptr patching to REV. A once i figure out how to develop cfw for it + if "A" in target.upper(): + patch_rev_a() + + if "B" in target.upper(): + patch_rev_b() + + +if __name__ == "__main__": + main() diff --git a/tests/test_combine.py b/tests/test_combine.py new file mode 100644 index 0000000..c7a6352 --- /dev/null +++ b/tests/test_combine.py @@ -0,0 +1,93 @@ +import hashlib +import importlib +import os +import tempfile +import unittest +from pathlib import Path + + +combine = importlib.import_module("combine") + + +def make_firmware(body): + header = bytearray(32) + header[8:12] = len(body).to_bytes(4, byteorder="little") + header[24:32] = hashlib.md5(body).digest()[:8] + return bytes(header) + body + + +class FirmwareValidationTests(unittest.TestCase): + def test_read_firmware_image_returns_valid_header_and_body(self): + body = b"firmware-body" + with tempfile.NamedTemporaryFile(delete=False) as tmp: + tmp.write(make_firmware(body)) + path = tmp.name + try: + header, read_body = combine.read_firmware_image(path) + finally: + os.unlink(path) + + self.assertEqual(len(header), 32) + self.assertEqual(read_body, body) + + def test_read_firmware_image_rejects_bad_body_length(self): + body = b"firmware-body" + image = bytearray(make_firmware(body)) + image[8:12] = (len(body) + 1).to_bytes(4, byteorder="little") + with tempfile.NamedTemporaryFile(delete=False) as tmp: + tmp.write(image) + path = tmp.name + try: + with self.assertRaisesRegex(ValueError, "length"): + combine.read_firmware_image(path) + finally: + os.unlink(path) + + def test_read_firmware_image_rejects_bad_body_hash(self): + body = b"firmware-body" + image = bytearray(make_firmware(body)) + image[24:32] = b"\x00" * 8 + with tempfile.NamedTemporaryFile(delete=False) as tmp: + tmp.write(image) + path = tmp.name + try: + with self.assertRaisesRegex(ValueError, "hash"): + combine.read_firmware_image(path) + finally: + os.unlink(path) + + def test_read_firmware_image_rejects_short_header(self): + with tempfile.NamedTemporaryFile(delete=False) as tmp: + tmp.write(b"short") + path = tmp.name + try: + with self.assertRaisesRegex(ValueError, "32-byte header"): + combine.read_firmware_image(path) + finally: + os.unlink(path) + + +class LoaderInputTests(unittest.TestCase): + def test_read_loader_binary_returns_existing_readable_file(self): + with tempfile.NamedTemporaryFile(delete=False) as tmp: + tmp.write(b"loader") + path = tmp.name + try: + self.assertEqual(combine.read_loader_binary(path), b"loader") + finally: + os.unlink(path) + + def test_read_loader_binary_rejects_missing_file(self): + path = Path(tempfile.gettempdir()) / "missing-playbrew-loader.bin" + with self.assertRaises(FileNotFoundError): + combine.read_loader_binary(path) + + +class PatchBoundsTests(unittest.TestCase): + def test_patch_rev_b_pointer_rejects_out_of_bounds_offset(self): + with self.assertRaisesRegex(ValueError, "outside firmware body"): + combine.patch_rev_b_pointer(bytearray(3), ptr_addr=0, update_func_addr=1) + + +if __name__ == "__main__": + unittest.main()