Harden tests flagged in code review
Strengthen weak-assertion / false-confidence tests:
- test_alfcrypto: replace the tautological ctx_init determinism check
with an independent golden vector, and add a golden Topaz decrypt
vector that does not rely on the test's own inverse helper (so a
systematic cipher bug is caught, not just round-trip symmetry). Pin
the PC1 bad-key assertion to match="Bad key length", and load
alfcrypto via the dedrm package so the test exercises the same module
object that topazextract/mobidedrm import.
- test_mobidedrm: pin the PC1 bad-key assertion to the guard message.
- test_topazextract: make the path-traversal regression rely on the
depth-independent positive oracle (the sanitised file must land inside
outdir, which fails against the pre-fix code) plus an exact-contents
check, instead of brittle parent-path negatives.
- test_erdr2pml: narrow the import skip to only the missing-cgi case so
a genuinely broken module fails loudly instead of silently skipping.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
+21
-4
@@ -6,7 +6,10 @@ import pytest
|
||||
|
||||
import dedrm_test_utils as U
|
||||
|
||||
alf = U.load("alfcrypto")
|
||||
# Load via the `dedrm` package so we exercise the same module object that
|
||||
# topazextract/mobidedrm import through `from .alfcrypto import ...`, rather
|
||||
# than a second top-level copy.
|
||||
alf = U.load("alfcrypto", package="dedrm")
|
||||
|
||||
|
||||
def test_pc1_roundtrip():
|
||||
@@ -18,7 +21,8 @@ def test_pc1_roundtrip():
|
||||
|
||||
|
||||
def test_pc1_rejects_bad_key_length():
|
||||
with pytest.raises(Exception):
|
||||
# Pin the specific guard, not just "some exception".
|
||||
with pytest.raises(Exception, match="Bad key length"):
|
||||
alf.Pukall_Cipher().PC1(b"\x00" * 8, b"data............", decryption=False)
|
||||
|
||||
|
||||
@@ -42,8 +46,21 @@ def test_topaz_cipher_roundtrip():
|
||||
assert recovered == plaintext
|
||||
|
||||
|
||||
def test_topaz_ctx_init_is_deterministic():
|
||||
assert alf.Topaz_Cipher().ctx_init(b"abc") == alf.Topaz_Cipher().ctx_init(b"abc")
|
||||
def test_topaz_ctx_init_golden():
|
||||
# Independent golden vector pins the key-schedule math; a bare determinism
|
||||
# check (ctx_init(x) == ctx_init(x)) would pass for any implementation.
|
||||
assert alf.Topaz_Cipher().ctx_init(b"abc") == [426181496, 3966955212]
|
||||
|
||||
|
||||
def test_topaz_decrypt_golden():
|
||||
# Golden oracle independent of the test's own inverse helper: a fixed
|
||||
# ciphertext + key must always decrypt to exactly these bytes. This catches
|
||||
# a systematic state-update bug that the round-trip test cannot.
|
||||
ctx = alf.Topaz_Cipher().ctx_init(b"topaz-key")
|
||||
out = alf.Topaz_Cipher().decrypt(bytes(range(32)), list(ctx)).encode("latin-1")
|
||||
assert out == bytes.fromhex(
|
||||
"e64653fbb4c4ad06981b51a0e53cd17ad9c7bc72932626ed35b935791f4fd769"
|
||||
)
|
||||
|
||||
|
||||
def test_pbkdf2_matches_hashlib():
|
||||
|
||||
@@ -13,9 +13,15 @@ import dedrm_test_utils as U
|
||||
try:
|
||||
erdr2pml = U.load("erdr2pml", package="dedrm")
|
||||
_skip_reason = None
|
||||
except Exception as exc: # pragma: no cover - depends on interpreter / deps
|
||||
erdr2pml = None
|
||||
_skip_reason = "erdr2pml not importable: {0}".format(exc)
|
||||
except ImportError as exc: # pragma: no cover - depends on interpreter / deps
|
||||
# Only tolerate the specific "no cgi backport" case (Python 3.13+ without
|
||||
# legacy-cgi). Any other import error means the module is genuinely broken
|
||||
# and must fail loudly rather than silently skip.
|
||||
if getattr(exc, "name", None) == "cgi":
|
||||
erdr2pml = None
|
||||
_skip_reason = "erdr2pml needs the 'cgi' module (install legacy-cgi on Python 3.13+)"
|
||||
else:
|
||||
raise
|
||||
|
||||
pytestmark = pytest.mark.skipif(erdr2pml is None, reason=_skip_reason or "")
|
||||
|
||||
|
||||
@@ -16,7 +16,7 @@ def test_pc1_roundtrip():
|
||||
|
||||
|
||||
def test_pc1_rejects_bad_key_length():
|
||||
with pytest.raises(Exception):
|
||||
with pytest.raises(Exception, match="Bad key length"):
|
||||
md.PC1(b"\x00" * 8, b"data............", decryption=False)
|
||||
|
||||
|
||||
|
||||
@@ -41,11 +41,16 @@ def test_extractfiles_strips_path_traversal(tmp_path):
|
||||
|
||||
book.extractFiles()
|
||||
|
||||
# The traversal sequence was stripped: nothing was written above outdir.
|
||||
# Primary oracle (independent of traversal depth): with the fix, basename()
|
||||
# strips every "../" so the record always lands INSIDE outdir. The pre-fix
|
||||
# code did os.path.join(outdir, "../../evil0000.dat"), writing OUTSIDE outdir,
|
||||
# so this assertion fails without the fix no matter how deep tmp_path nests.
|
||||
assert (outdir / "evil0000.dat").read_bytes() == b"PWNED"
|
||||
# outdir contains exactly that one sanitised file, nothing unexpected.
|
||||
assert sorted(p.name for p in outdir.iterdir()) == ["evil0000.dat"]
|
||||
# Defense in depth: nothing escaped to the dirs the "../../" payload targets.
|
||||
assert not (tmp_path / "evil0000.dat").exists()
|
||||
assert not (tmp_path.parent / "evil0000.dat").exists()
|
||||
# The (sanitised) write landed safely inside outdir.
|
||||
assert (outdir / "evil0000.dat").read_bytes() == b"PWNED"
|
||||
|
||||
|
||||
def test_extractfiles_writes_normal_record(tmp_path):
|
||||
|
||||
Reference in New Issue
Block a user