bugfix: enable disabled 7z magic check in check_file_headers
What changed, and why it matters
This commit fixes a bug where COLDCARD backup files in 7z format were not properly checked for the correct file header ('magic bytes'). Because the check was accidentally disabled, the device could accept backup files with wrong or corrupted headers. The fix turns the check back on and adds tests to make sure bad headers are rejected with a clear error message.
Treat this as a low-to-moderate security fix and include it in the next firmware release. Review other validation helpers for similar missing-call bugs. No immediate user action is required beyond installing the updated firmware once released.
Security signals we found
Validation bypass due to missing method call parentheses
File format header integrity check restored
Backup import path now rejects malformed/crafted 7z containers
Regression tests added for invalid magic, major, and minor version fields
Evidence from the diff
In shared/compat7z.py, check_file_headers() used if not fh.has_good_magic: which tests the truthiness of a bound method object rather than calling it. In Python a bound method is always truthy, so the magic validation was effectively bypassed. The patch changes this to if not fh.has_good_magic():, restoring the intended header validation. A regression test is added that corrupts magic bytes, major version, and minor version fields of a sample backup.7z and asserts that verify_backup_file raises an error and shows ‘Bad magic bytes’.
Changed components
shared/compat7z.py:check_file_headers()COLDCARD backup restore flow7z archive compatibility layerInspect captured patch +53 / −2
diff --git a/releases/Next-ChangeLog.md b/releases/Next-ChangeLog.md
index b829d2b..c7ea947 100644
--- a/releases/Next-ChangeLog.md
+++ b/releases/Next-ChangeLog.md
@@ -5,6 +5,7 @@ This lists the new changes that have not yet been published in a normal release.
# Shared Improvements - Both Mk and Q
- Bugfix: Delta Mode Trick PIN was never restored from backup
+- Bugfix: Proper error message for incorrect 7z headers
# Mk Specific Changes
diff --git a/shared/compat7z.py b/shared/compat7z.py
index 3034d1c..55b6b88 100644
--- a/shared/compat7z.py
+++ b/shared/compat7z.py
@@ -100,7 +100,7 @@ def check_file_headers(f):
# assume f is seekable
fh = FileHeader.read(f)
- if not fh.has_good_magic:
+ if not fh.has_good_magic():
raise ValueError("Bad magic bytes")
# read only first header
diff --git a/testing/test_backup.py b/testing/test_backup.py
index 40eb3cb..21062b2 100644
--- a/testing/test_backup.py
+++ b/testing/test_backup.py
@@ -2,7 +2,7 @@
#
# Testing backups.
#
-import pytest, time, json, os, shutil, re
+import pytest, time, json, os, shutil, re, struct
from constants import simulator_fixed_words, simulator_fixed_tprv
from charcodes import KEY_QR
from bip32 import BIP32Node
@@ -842,4 +842,54 @@ def test_backup_long_name_display(fname, goto_home, pick_menu_item, need_keypres
press_cancel()
+
+def test_header_magic_check(microsd_path, src_root_dir, verify_backup_file, cap_story):
+ fname = "backup.7z"
+ fn = microsd_path(fname)
+
+ with open(f'{src_root_dir}/docs/backup.7z', "rb") as f:
+ conts = f.read()
+
+ # from shared/compat7z.py
+ magic, major, minor, crc = struct.unpack('<6sBBL', conts[:12])
+ assert magic == b"7z\xbc\xaf'\x1c"
+ assert major == 0
+ assert minor >= 3
+
+ # invalid magic
+ with open(fn, "wb") as f:
+ f.write(b"8z\xbc\xaf'\x1c")
+ f.write(conts[6:])
+
+ with pytest.raises(AssertionError):
+ verify_backup_file(fname)
+
+ title, story = cap_story()
+ assert "Bad magic bytes" in story
+
+ # invalid major
+ with open(fn, "wb") as f:
+ f.write(b"7z\xbc\xaf'\x1c")
+ f.write(bytes([1])) # major has to be 0
+ f.write(conts[7:])
+
+ with pytest.raises(AssertionError):
+ verify_backup_file(fname)
+
+ title, story = cap_story()
+ assert "Bad magic bytes" in story
+
+ # invalid minor
+ with open(fn, "wb") as f:
+ f.write(b"7z\xbc\xaf'\x1c")
+ f.write(bytes([0]))
+ f.write(bytes([2])) # cannot be smaller than 3
+ f.write(conts[8:])
+
+ with pytest.raises(AssertionError):
+ verify_backup_file(fname)
+
+ title, story = cap_story()
+ assert "Bad magic bytes" in story
+
# EOF
Why this scored 34/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.