From 8d8b496044a0c546b281a619f680ede0ceaecdbd Mon Sep 17 00:00:00 2001 From: scgbckbone Date: Thu, 23 Oct 2025 14:19:48 +0200 Subject: [PATCH] older miniscript fragment validation --- releases/EdgeChangeLog.md | 3 ++- shared/miniscript.py | 17 ++++++++++++++--- testing/test_miniscript.py | 32 ++++++++++++++++++++++++++------ 3 files changed, 42 insertions(+), 10 deletions(-) diff --git a/releases/EdgeChangeLog.md b/releases/EdgeChangeLog.md index 215c52fd..27b0617c 100644 --- a/releases/EdgeChangeLog.md +++ b/releases/EdgeChangeLog.md @@ -35,7 +35,8 @@ This lists the changes in the most recent EDGE firmware, for each hardware platf with BIP-380 extended key expression `[xfp/origin_path]xpub`. - Bugfix: Disjoint derivation in miniscript wallets - Bugfix: Disallow P2SH legacy miniscript -- Bugfix: Do not allow to import miniscript with `older(N > 65535)` +- Bugfix: Do not allow to import miniscripts with relative lock without consensus meaning. + Only allow to import block-based in range `older(1 - 65535)` & time-based in range `older(4194305 - 4259839)` # Mk4 Specific Changes diff --git a/shared/miniscript.py b/shared/miniscript.py index b751ee4a..497d7bc3 100644 --- a/shared/miniscript.py +++ b/shared/miniscript.py @@ -280,14 +280,17 @@ class After(OneArg): def verify(self): super().verify() - assert 1 <= self.arg.num < 0x80000000, "%s out of range [1, 2147483647]" % self.NAME + assert 0 < self.arg.num < 0x80000000, "%s out of range [1, 2147483647]" % self.NAME def __len__(self): return self.len_args() + 1 -class Older(After): +class Older(OneArg): # CHECKSEQUENCEVERIFY NAME = "older" + ARGCLS = Number + TYPE = "B" + PROPS = "z" def inner_compile(self): return self.carg + b"\xb2" @@ -296,7 +299,15 @@ class Older(After): super().verify() # not consensus valid # https://github.com/bitcoin/bitcoin/pull/33135 older(65536) is equivalent to older(1) - assert self.arg.num < 0x10000, "%s out of range [1, 65535]" % self.NAME + if self.arg.num & (1 << 22): + # time-based + assert 0x400000 < self.arg.num < 0x410000, "Time-based %s out of range [4194305, 4259839]" % self.NAME + else: + # block-based + assert 0 < self.arg.num < 0x10000, "Block-based %s out of range [1, 65535]" % self.NAME + + def __len__(self): + return self.len_args() + 1 class Sha256(OneArg): # SIZE <32> EQUALVERIFY SHA256 EQUAL diff --git a/testing/test_miniscript.py b/testing/test_miniscript.py index 987ffe38..6ce24e2b 100644 --- a/testing/test_miniscript.py +++ b/testing/test_miniscript.py @@ -3367,7 +3367,15 @@ def test_legacy_sh_miniscript(offer_minsc_import, press_select, create_core_wall assert "Miniscript in legacy P2SH not allowed" in str(e) -@pytest.mark.parametrize("lock", ["older", "after"]) +@pytest.mark.parametrize("lock", [ + ("older", 0), + ("after", 0), + ("older", 65536), + ("after", 2147483648), + # time-based relative locks + ("older", 4194304), + ("older", 4259840), +]) def test_timelocks_without_consesnsus_meaning(lock, clear_miniscript, goto_home, get_cc_key, offer_minsc_import, press_select): goto_home() @@ -3375,10 +3383,8 @@ def test_timelocks_without_consesnsus_meaning(lock, clear_miniscript, goto_home, policy = "and_v(v:pk(@0/<0;1>/*),locktime())" # not allowed to import on CC - if lock == "older": - to_replace = "older(65536)" - else: - to_replace = "after(2147483648)" + _type, val = lock + to_replace = f"{_type}({val})" policy = policy.replace("locktime()", to_replace) @@ -3392,7 +3398,21 @@ def test_timelocks_without_consesnsus_meaning(lock, clear_miniscript, goto_home, with pytest.raises(Exception) as e: offer_minsc_import(json.dumps(dict(name=wname, desc=desc))) - assert f"{lock} out of range [1, {(2**16)-1 if (lock == 'older') else (2**31)-1}]" in e.value.args[0] + if _type == "older": + if val & (1 << 22): + what = "Time-based " + x = 4194305 + y = 4259839 + else: + what = "Block-based " + x = 1 + y = (2**16)-1 + else: + what = "" + x = 1 + y = (2**31)-1 + + assert f"{what}{lock[0]} out of range [{x}, {y}]" in e.value.args[0] press_select() # EOF \ No newline at end of file