fix: nl_mkimage Verifikation robuster gegen Edge Cases und Misuse
Critical 1: path.read_bytes() fiel auf nonexistent file oder Directory mit Traceback statt einer Fehlerzeile. Jetzt OSError-Familie abgefangen. Critical 2: --device/--version mit Leerzeichen wurden stillschweigend akzeptiert; das zerlegt das Ausgabeformat und bricht Task 4s Skript. Jetzt whitespace beim Bauen abgelehnt. Important 1: Gerätefeld muss in C wie in Python null-terminiert sein. Die Firmware checkt das unabhängig von --expect-device; Python nur mit Flag. Jetzt Terminierung immer verlangt. Important 2: Modi waren nicht exklusiv; --verify mit Baumodus-Argumenten wurde stillschweigend ignoriert. Jetzt Fehler statt falschem Vertrauen. Neue Tests für alle vier Szenarien + nonexistent file, directory, empty file, Trailer ohne Payload, Mode-Mischung. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Haiku 4.5
parent
0ba269a39a
commit
df8e796f1e
@@ -19,12 +19,18 @@ def field(value: str, name: str) -> bytes:
|
|||||||
raw = value.encode()
|
raw = value.encode()
|
||||||
if len(raw) >= FIELD:
|
if len(raw) >= FIELD:
|
||||||
sys.exit(f"error: {name} '{value}' is too long (max {FIELD - 1} bytes)")
|
sys.exit(f"error: {name} '{value}' is too long (max {FIELD - 1} bytes)")
|
||||||
|
if any(c.isspace() for c in value):
|
||||||
|
sys.exit(f"error: {name} '{value}' contains whitespace")
|
||||||
return raw.ljust(FIELD, b"\x00")
|
return raw.ljust(FIELD, b"\x00")
|
||||||
|
|
||||||
|
|
||||||
def verify(path: Path, expect_device: str | None) -> int:
|
def verify(path: Path, expect_device: str | None) -> int:
|
||||||
"""Checks an _ota.img the way the firmware does before accepting it."""
|
"""Checks an _ota.img the way the firmware does before accepting it."""
|
||||||
data = path.read_bytes()
|
try:
|
||||||
|
data = path.read_bytes()
|
||||||
|
except OSError as e:
|
||||||
|
print(f"error: {e}", file=sys.stderr)
|
||||||
|
return 1
|
||||||
if len(data) <= TRAILER_SIZE:
|
if len(data) <= TRAILER_SIZE:
|
||||||
print(f"error: {path} is too short to carry a trailer", file=sys.stderr)
|
print(f"error: {path} is too short to carry a trailer", file=sys.stderr)
|
||||||
return 1
|
return 1
|
||||||
@@ -36,7 +42,12 @@ def verify(path: Path, expect_device: str | None) -> int:
|
|||||||
print(f"error: {path} has trailer version {trailer[4]}, expected {TRAILER_VERSION}",
|
print(f"error: {path} has trailer version {trailer[4]}, expected {TRAILER_VERSION}",
|
||||||
file=sys.stderr)
|
file=sys.stderr)
|
||||||
return 1
|
return 1
|
||||||
device = trailer[5:21].rstrip(b"\x00").decode("ascii", "replace")
|
# Device field must be null-terminated within its 16 bytes (same as firmware check)
|
||||||
|
device_field = trailer[5:21]
|
||||||
|
if b"\x00" not in device_field:
|
||||||
|
print(f"error: {path} has unterminated device field", file=sys.stderr)
|
||||||
|
return 1
|
||||||
|
device = device_field.rstrip(b"\x00").decode("ascii", "replace")
|
||||||
version = trailer[21:37].rstrip(b"\x00").decode("ascii", "replace")
|
version = trailer[21:37].rstrip(b"\x00").decode("ascii", "replace")
|
||||||
if expect_device is not None and device != expect_device:
|
if expect_device is not None and device != expect_device:
|
||||||
print(f"error: {path} carries device '{device}', expected '{expect_device}'",
|
print(f"error: {path} carries device '{device}', expected '{expect_device}'",
|
||||||
@@ -61,13 +72,22 @@ def main() -> None:
|
|||||||
a = p.parse_args()
|
a = p.parse_args()
|
||||||
|
|
||||||
if a.verify is not None:
|
if a.verify is not None:
|
||||||
|
# Verify mode: check that build arguments are not provided
|
||||||
|
build_args = ("image", "device", "version", "out")
|
||||||
|
if any(getattr(a, name) is not None for name in build_args):
|
||||||
|
p.error("--verify cannot be combined with build mode arguments (--image, --device, --version, --out)")
|
||||||
raise SystemExit(verify(a.verify, a.expect_device))
|
raise SystemExit(verify(a.verify, a.expect_device))
|
||||||
|
|
||||||
|
# Build mode: require all arguments
|
||||||
for name in ("image", "device", "version", "out"):
|
for name in ("image", "device", "version", "out"):
|
||||||
if getattr(a, name) is None:
|
if getattr(a, name) is None:
|
||||||
p.error(f"--{name} is required unless --verify is given")
|
p.error(f"--{name} is required unless --verify is given")
|
||||||
|
|
||||||
data = a.image.read_bytes()
|
try:
|
||||||
|
data = a.image.read_bytes()
|
||||||
|
except OSError as e:
|
||||||
|
sys.exit(f"error: {e}")
|
||||||
|
|
||||||
if len(data) > TRAILER_SIZE and data[-TRAILER_SIZE : -TRAILER_SIZE + 4] == b"NINL":
|
if len(data) > TRAILER_SIZE and data[-TRAILER_SIZE : -TRAILER_SIZE + 4] == b"NINL":
|
||||||
sys.exit("error: image already carries a trailer")
|
sys.exit("error: image already carries a trailer")
|
||||||
|
|
||||||
|
|||||||
@@ -100,3 +100,111 @@ def test_verify_rejects_a_file_shorter_than_the_trailer(tmp_path):
|
|||||||
p.write_bytes(b"NINL" + b"\x00" * 10)
|
p.write_bytes(b"NINL" + b"\x00" * 10)
|
||||||
r = verify(p)
|
r = verify(p)
|
||||||
assert r.returncode == 1
|
assert r.returncode == 1
|
||||||
|
|
||||||
|
|
||||||
|
def test_verify_rejects_nonexistent_file(tmp_path):
|
||||||
|
p = tmp_path / "does_not_exist.img"
|
||||||
|
r = verify(p)
|
||||||
|
assert r.returncode == 1
|
||||||
|
assert r.stdout == ""
|
||||||
|
assert len(r.stderr.splitlines()) == 1
|
||||||
|
assert "error:" in r.stderr.lower()
|
||||||
|
|
||||||
|
|
||||||
|
def test_verify_rejects_directory(tmp_path):
|
||||||
|
d = tmp_path / "dir"
|
||||||
|
d.mkdir()
|
||||||
|
r = verify(d)
|
||||||
|
assert r.returncode == 1
|
||||||
|
assert r.stdout == ""
|
||||||
|
assert len(r.stderr.splitlines()) == 1
|
||||||
|
assert "error:" in r.stderr.lower()
|
||||||
|
|
||||||
|
|
||||||
|
def test_verify_rejects_empty_file(tmp_path):
|
||||||
|
p = tmp_path / "empty.img"
|
||||||
|
p.write_bytes(b"")
|
||||||
|
r = verify(p)
|
||||||
|
assert r.returncode == 1
|
||||||
|
assert "trailer" in r.stderr.lower()
|
||||||
|
|
||||||
|
|
||||||
|
def test_verify_rejects_file_with_only_trailer_no_payload(tmp_path):
|
||||||
|
p = tmp_path / "only_trailer.img"
|
||||||
|
trailer = bytearray(b"\xff" * 512)
|
||||||
|
trailer[0:4] = b"NINL"
|
||||||
|
trailer[4] = 1
|
||||||
|
trailer[5:21] = b"test\x00" + b"\x00" * 11
|
||||||
|
trailer[21:37] = b"1.0.0\x00" + b"\x00" * 10
|
||||||
|
trailer[37:69] = hashlib.sha256(b"").digest()
|
||||||
|
p.write_bytes(bytes(trailer))
|
||||||
|
r = verify(p)
|
||||||
|
assert r.returncode == 1
|
||||||
|
assert "trailer" in r.stderr.lower()
|
||||||
|
|
||||||
|
|
||||||
|
def test_build_rejects_device_with_whitespace(tmp_path):
|
||||||
|
src = tmp_path / "fw_gz.img"
|
||||||
|
src.write_bytes(b"x")
|
||||||
|
r = subprocess.run(
|
||||||
|
[sys.executable, str(SCRIPT), "--image", str(src), "--device", "dev ice",
|
||||||
|
"--version", "1.0", "--out", str(tmp_path / "o.img")],
|
||||||
|
capture_output=True, text=True,
|
||||||
|
)
|
||||||
|
assert r.returncode != 0
|
||||||
|
assert "whitespace" in r.stderr
|
||||||
|
|
||||||
|
|
||||||
|
def test_build_rejects_version_with_whitespace(tmp_path):
|
||||||
|
src = tmp_path / "fw_gz.img"
|
||||||
|
src.write_bytes(b"x")
|
||||||
|
r = subprocess.run(
|
||||||
|
[sys.executable, str(SCRIPT), "--image", str(src), "--device", "device",
|
||||||
|
"--version", "1.0 beta", "--out", str(tmp_path / "o.img")],
|
||||||
|
capture_output=True, text=True,
|
||||||
|
)
|
||||||
|
assert r.returncode != 0
|
||||||
|
assert "whitespace" in r.stderr
|
||||||
|
|
||||||
|
|
||||||
|
def test_verify_rejects_unterminated_device_field(tmp_path):
|
||||||
|
p = tmp_path / "bad_device.img"
|
||||||
|
payload = b"x" * 1000
|
||||||
|
trailer = bytearray(b"\xff" * 512)
|
||||||
|
trailer[0:4] = b"NINL"
|
||||||
|
trailer[4] = 1
|
||||||
|
# Fill entire device field with no null terminator
|
||||||
|
trailer[5:21] = b"a" * 16
|
||||||
|
trailer[21:37] = b"1.0.0\x00" + b"\x00" * 10
|
||||||
|
trailer[37:69] = hashlib.sha256(payload).digest()
|
||||||
|
p.write_bytes(payload + bytes(trailer))
|
||||||
|
r = verify(p)
|
||||||
|
assert r.returncode == 1
|
||||||
|
assert "unterminated" in r.stderr.lower()
|
||||||
|
|
||||||
|
|
||||||
|
def test_verify_rejects_unterminated_device_field_without_expect_device(tmp_path):
|
||||||
|
# Same as above but without --expect-device to ensure the check happens regardless
|
||||||
|
p = tmp_path / "bad_device2.img"
|
||||||
|
payload = b"x" * 1000
|
||||||
|
trailer = bytearray(b"\xff" * 512)
|
||||||
|
trailer[0:4] = b"NINL"
|
||||||
|
trailer[4] = 1
|
||||||
|
# Fill entire device field with no null terminator
|
||||||
|
trailer[5:21] = b"b" * 16
|
||||||
|
trailer[21:37] = b"2.0.0\x00" + b"\x00" * 10
|
||||||
|
trailer[37:69] = hashlib.sha256(payload).digest()
|
||||||
|
p.write_bytes(payload + bytes(trailer))
|
||||||
|
r = verify(p)
|
||||||
|
assert r.returncode == 1
|
||||||
|
assert "unterminated" in r.stderr.lower()
|
||||||
|
|
||||||
|
|
||||||
|
def test_verify_with_build_arguments_is_an_error(tmp_path):
|
||||||
|
_, out = build(tmp_path)
|
||||||
|
r = subprocess.run(
|
||||||
|
[sys.executable, str(SCRIPT), "--verify", str(out), "--device", "x"],
|
||||||
|
capture_output=True, text=True,
|
||||||
|
)
|
||||||
|
assert r.returncode != 0
|
||||||
|
assert "cannot be combined" in r.stderr
|
||||||
|
|||||||
Reference in New Issue
Block a user