From fe83e2b5d28df39f34be980ecf1f86ff32afdbbc Mon Sep 17 00:00:00 2001 From: Ferdinando Ametrano Date: Thu, 13 Aug 2026 18:39:08 +0200 Subject: [PATCH] Refuse the malformed input twelve places answered anyway Not an exception of the wrong class: no exception at all, and a value the caller goes on to use. `sig_hash.taproot` hashed an input index past the end of the vin, two out-of-range indexes giving two different hashes; `bech32.encode` indexed its alphabet from the end for a negative digit and wrote another address; `input_script_sig` read -1 as the last leaf and proved it; `indexes_from_der_path` handed `[-5]` back; `pub_key_derivation_tweaks` never looked at the key for a path of no steps; `miniscript_solver` solved the input at the other end; `weight_estimate` estimated an incoherent psbt; `number_theory`'s five answered a float with a float; `int_from_json_number` truncated 1.5; `bin_str_entropy_from_wordlist_indexes` carried an index no word answers to; `tx_for_network` baked a network no table has into every output; `assert_valid_control_block` measured characters. The first slice of issue #744, and the one it puts first: the only category of that census that can cost a user money. Two tests asserted the defect and now assert the fix, both in bip32: `derive(xprv, 2**32)` was an `OverflowError` and is a `BTClibValueError`, and `BIP32KeyOrigin`'s out-of-range index is refused by the path reader before `assert_valid` sees it -- which still asks, the field being a list the frozen dataclass cannot stop an append to. The bool half of the six new integer guards goes in `tests/integer_policy_test.py`, beside the twenty-five parameters already there rather than in six files. Closes part of #744. --- CHANGELOG.md | 64 +++++++++++++++++++++++++++ btclib/bech32.py | 24 +++++++++- btclib/bip32/bip32.py | 27 ++++++----- btclib/bip32/der_path.py | 39 ++++++++++------ btclib/descriptors/descriptors.py | 6 +++ btclib/fetch/fetcher.py | 11 ++++- btclib/mnemonic/entropy.py | 14 +++++- btclib/number_theory.py | 49 +++++++++++++++++++- btclib/psbt/psbt.py | 6 +++ btclib/script/sig_hash.py | 21 ++++++++- btclib/script/taproot.py | 25 ++++++++--- btclib/utils.py | 19 +++++++- tests/bech32_test.py | 27 ++++++++++- tests/bip32/bip32_test.py | 33 +++++++++++++- tests/bip32/der_path_test.py | 30 +++++++++++++ tests/bip32/key_origin_test.py | 16 ++++++- tests/descriptors/miniscript_test.py | 19 ++++++++ tests/fetch/fetcher_test.py | 27 ++++++++++- tests/integer_policy_test.py | 24 +++++++++- tests/mnemonic/entropy_test.py | 27 ++++++++++- tests/number_theory_test.py | 41 ++++++++++++++++- tests/psbt/psbt_size_test.py | 22 +++++++++ tests/script/sig_hash_taproot_test.py | 44 +++++++++++++++++- tests/script/taproot_test.py | 58 +++++++++++++++++++++++- tests/utils_test.py | 35 ++++++++++++++- 25 files changed, 655 insertions(+), 53 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c9e112ee..da6c32ca1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1658,6 +1658,70 @@ documented at release-notes length in the first place, and are still in ### The public API and the module layout +- **Twelve places answered a malformed argument instead of refusing it** + (issue #744): no exception of the wrong class, no exception at all -- + a hash, an address, an entropy, a weight, a residue, handed back for an + input that names nothing. They are unrelated bugs of one shape, which + is why they are one entry: a check written for one spelling of an + argument, or one branch of a function, and not for the others. + + - `script.sig_hash.taproot` hashed an `input_index` past the end of the + vin. BIP341's SigMsg commits to the index itself, and outside the + ANYONECANPAY branch nothing dereferences it, so indexes 99 and 100 on + a two-input transaction produced two *different* 32-byte hashes, both + returned. The bound existed in the SIGHASH_SINGLE branch alone, and + against the vout. + - `script.taproot.input_script_sig` read `script_num` as a list index, + so -1 selected the last leaf and -2 the one before it, each with a + control block that correctly proves the leaf nobody asked for. + - `script.taproot.assert_valid_control_block` measured `len` of + whatever it was handed: `"é" * 33` is 33 characters and 66 octets of + UTF-8, and passed as a control block size. The octets are taken first + now, as `check_output_pubkey` takes them on the same argument. + - `bech32.encode` indexed its alphabet with the digits it was given, so + a negative one counted from the end of the alphabet and wrote a + different address, correctly checksummed and silent. A digit above 31 + raised `IndexError`, a `LookupError` and so outside every `except + BTClibValueError`. + - `bip32.der_path.indexes_from_der_path` enforced `0 <= index < + 0x80000000` for the text spelling of a path and nothing for the + others: `indexes_from_der_path([-5])` answered `[-5]`. The + `OverflowError` that `derive`, `bytes_from_der_path` and + `BIP32KeyOrigin.serialize` then raised is fixed with it, an + `ArithmeticError` being no better than a wrong answer for a caller + filtering bad input. + - `bip32.pub_key_derivation_tweaks` skipped its whole body for a path + of no steps, so 33 bytes that are no public key came back as `[]` -- + the answer a caller reads as "derived, nothing to apply". `[]` is + right for an empty path and wrong for a non-point. + - `descriptors.miniscript_solver` read `psbt.inputs[vin_i]` unchecked, + so a negative index solved the input at the other end and answered a + witness for it. Its siblings `update_psbt_input` and + `update_psbt_output` carry the guard, with the comment saying why. + - `Psbt.weight_estimate` -- and `estimated_weight` and + `estimated_vsize` through it -- estimated an incoherent psbt rather + than refusing, alone among the public methods that read a psbt's own + data. A weight is what a fee is computed from. + - `number_theory`'s `xgcd`, `mod_inv`, `legendre_symbol`, `mod_sqrt` + and `tonelli` ran on a float and answered one: `mod_inv(3.0, 7)` was + `5.0`, out of a signature that says `int`. A modulus of zero raised + `ZeroDivisionError`, which `except ValueError` does not catch. The + guard is `var_int.serialize`'s, and it costs a fraction of a percent + of the arithmetic it stands in front of. + - `utils.int_from_json_number` truncated: `1.5` was 1, silently and to + a number the caller did write. 1.0 is the json spelling of 1 and + still coerces; `nan` and `inf` are no more whole than 1.5. + - `mnemonic.entropy.bin_str_entropy_from_wordlist_indexes` accepted an + index no word answers to. Base-`base` arithmetic has no out of range: + 2048 in a 2048-word list is a carry into the digit above it, so the + entropy came back wrong rather than refused. + - `fetch.fetcher.tx_for_network` compared the name against `"mainnet"` + as text and labelled every output with whatever else it was given, + `check_validity=False` throughout: a network no table has was baked + into the transaction handed back, to surface far from the call. The + name is resolved now, which also makes `" MainNet "` the + short-circuit it always should have been. + - **`network_from_name` is the one place a network name becomes a `Network`** (issue #744), and fourteen call sites that indexed `NETWORKS[network]` raw go through it. A name no network has was a bare diff --git a/btclib/bech32.py b/btclib/bech32.py index 84782fa89..716abd76a 100644 --- a/btclib/bech32.py +++ b/btclib/bech32.py @@ -58,7 +58,8 @@ from operator import xor from btclib.alias import String -from btclib.exceptions import BTClibValueError +from btclib.exceptions import BTClibTypeError, BTClibValueError +from btclib.utils import is_integer __all__ = [ "decode", @@ -180,7 +181,26 @@ def decode(bech: String, m: int | None = None) -> tuple[str, list[int]]: def encode(hrp: str, data: list[int], m: int | None = None) -> bytes: - """Compute a bech32 string given HRP and data values.""" + """Compute a bech32 string given HRP and data values. + + Every value is one 5-bit digit, and each is checked rather than left + to the alphabet lookup to fail: ``_ALPHABET[-1]`` is "l" and + ``_ALPHABET[-32]`` is "q", Python indexing from the end, so a + negative digit writes a *different address* and says nothing at all. + A digit above 31 at least raises, and raises `IndexError`; a float + raises `TypeError`. Neither is caught by the `except + BTClibValueError` this library invites. + + The pair of checks walks the digits a second time, which is a + fraction of what encoding them costs and a smaller fraction of the + key derivation that produced them -- an address is encoded once, + never in an inner loop. + """ + for d in data: + if not is_integer(d): + raise BTClibTypeError(f"invalid 5-bit value type: {type(d).__name__}") + if not 0 <= d < 32: + raise BTClibValueError(f"invalid 5-bit value: {d}") m = _m_from_wit_ver(data) if m is None else m combined = data + _create_checksum(hrp, data, m) s = f"{hrp}1" + "".join(_ALPHABET[d] for d in combined) diff --git a/btclib/bip32/bip32.py b/btclib/bip32/bip32.py index eb562fc7e..58a34eb75 100644 --- a/btclib/bip32/bip32.py +++ b/btclib/bip32/bip32.py @@ -573,17 +573,24 @@ def pub_key_derivation_tweaks( if any(index >= _HARDENED_OFFSET for index in indexes): raise BTClibValueError("invalid hardened derivation from public key") - tweaks: list[bytes] = [] - if indexes: - # one parse for the whole path rather than one per index: each - # step still needs its own serialized key, to hash into the next - # tweak, but not a fresh parse of the bytes the step before it - # just serialized -- PubkeyTweakChain holds the point in between + # one parse for the whole path rather than one per index: each step + # still needs its own serialized key, to hash into the next tweak, + # but not a fresh parse of the bytes the step before it just + # serialized -- PubkeyTweakChain holds the point in between. + # Outside the loop and not inside an `if indexes:`, so that a path of + # no steps is the one spelling of this call that still looks at the + # key it was handed: [] is the right answer for it, and the right + # answer for 33 bytes that are not a point is no answer + try: chain = libsecp256k1_keys.PubkeyTweakChain(key) - for index in indexes: - offset, code = _pub_key_offset(code, key, index) - tweaks.append(offset.to_bytes(32, byteorder="big")) - key = chain.tweak_add(offset, compressed=True) + except ValueError as e: + raise BTClibValueError(f"invalid public key: {key.hex()}") from e + + tweaks: list[bytes] = [] + for index in indexes: + offset, code = _pub_key_offset(code, key, index) + tweaks.append(offset.to_bytes(32, byteorder="big")) + key = chain.tweak_add(offset, compressed=True) return tweaks diff --git a/btclib/bip32/der_path.py b/btclib/bip32/der_path.py index 693008893..06d214fe1 100644 --- a/btclib/bip32/der_path.py +++ b/btclib/bip32/der_path.py @@ -91,6 +91,27 @@ def _index_and_hardening_from_str(s: str, *, bip380_enforced: bool) -> tuple[int return index + (_HARDENED_OFFSET if hardening else 0), hardening +def _assert_valid_index(i: int) -> None: + """Refuse anything one step of a BIP32 path cannot be. + + A step is one of the 2**32 indexes, hardened or not. What makes the + check worth its own name is that the two places needing it fail + differently without it: writing a path out reaches + `int.to_bytes(4, signed=False)` and an out-of-range index surfaces + there as `OverflowError`, which is an `ArithmeticError` and so + outside every `except ValueError` written against this library, + while `indexes_from_der_path` hands its list straight back and + answers `[-5]` for `[-5]` -- no error at all. + + A bool is no index either: `True` is not the first child of + anything, and `str(True)` is "True" where a path step wants "1". + """ + if not is_integer(i): + raise BTClibTypeError(f"invalid derivation index type: {type(i).__name__}") + if not 0 <= i <= 0xFFFFFFFF: + raise BTClibValueError(f"invalid index: {i}") + + def int_from_index_str(s: str, *, bip380_enforced: bool = False) -> int: """Return one path step as its index: "0h" is 0x80000000. @@ -113,17 +134,12 @@ def str_from_index_int(i: int, hardening: str = _HARDENING) -> str: """ if hardening not in _BIP380_HARDENINGS: raise BTClibValueError(f"invalid hardening symbol: {hardening}") - # reachable without indexes_from_der_path, and str(True) is "True": a - # path step of a boolean would be a path nothing derives - if not is_integer(i): - raise BTClibTypeError(f"invalid derivation index type: {type(i).__name__}") + _assert_valid_index(i) # int() of an int, because an IntEnum is one and str() of an IntEnum is # its *name* up to Python 3.10 -- "Sighash.ALL" where a path step wants # "1". Accepting a deliberate integer subclass, which is what - # is_integer above is for, means answering with the number it is + # `is_integer` is for, means answering with the number it is index = int(i) - if not 0 <= index <= 0xFFFFFFFF: - raise BTClibValueError(f"invalid index: {index}") if index < _HARDENED_OFFSET: return str(index) return str(index - _HARDENED_OFFSET) + hardening @@ -208,9 +224,7 @@ def hardenings_from_der_path( def _indexes_from_der_path(der_path: Sequence[int] | int | bytes) -> list[int]: """Return the indexes of every DerPath spelling that is not a string.""" if isinstance(der_path, int): - if not is_integer(der_path): - err_msg = f"invalid derivation index type: {type(der_path).__name__}" - raise BTClibTypeError(err_msg) + _assert_valid_index(der_path) return [der_path] if isinstance(der_path, bytes): @@ -224,12 +238,9 @@ def _indexes_from_der_path(der_path: Sequence[int] | int | bytes) -> list[int]: # an iterable of int, and of int alone: int() here would coerce a bool # into the index one, where the annotation already says Sequence[int] - # and a bool is no index -- `True` is not the first child of anything indexes = list(der_path) for index in indexes: - if not is_integer(index): - err_msg = f"invalid derivation index type: {type(index).__name__}" - raise BTClibTypeError(err_msg) + _assert_valid_index(index) return indexes diff --git a/btclib/descriptors/descriptors.py b/btclib/descriptors/descriptors.py index f9c5d61d0..6b291328a 100644 --- a/btclib/descriptors/descriptors.py +++ b/btclib/descriptors/descriptors.py @@ -2563,6 +2563,12 @@ def miniscript_solver(psbt: Psbt, vin_i: int) -> tuple[bytes, Witness] | None: network refuses after the transaction is broadcast rather than one this refuses while it is built. """ + # the guard `update_psbt_input` carries, for the same reason: an + # IndexError out of a public function is not an answer, and a + # negative index would quietly solve the input at the other end -- + # with a witness this one's script does not satisfy + if not 0 <= vin_i < len(psbt.inputs): + raise BTClibValueError(f"invalid input index: {vin_i}") psbt_in = psbt.inputs[vin_i] if not psbt_in.witness_script: return None diff --git a/btclib/fetch/fetcher.py b/btclib/fetch/fetcher.py index 1f43bbd9f..5e7d9c717 100644 --- a/btclib/fetch/fetcher.py +++ b/btclib/fetch/fetcher.py @@ -32,7 +32,7 @@ HttpError, RpcError, ) -from btclib.network import NETWORKS +from btclib.network import NETWORKS, network_from_name from btclib.script import ScriptPubKey from btclib.tx import OutPoint, Tx, TxOut from btclib.utils import bytes_from_octets @@ -125,8 +125,15 @@ def tx_for_network(tx: Tx, network: str) -> Tx: transaction equal to its argument, the label being the only thing it touches. The bytes are untouched in every case, `ScriptPubKey` serializing the script alone. + + The name is resolved and not compared as text. Resolving refuses a + network no table has, which every `check_validity=False` below would + otherwise write into the transaction handed back, to surface far + from here as whatever went on to render an address; and it answers + the same for " MainNet " as for "mainnet", where a comparison would + relabel every output instead of returning the transaction as it is. """ - if network == "mainnet": + if network_from_name(network) == NETWORKS["mainnet"]: return tx vout = [ TxOut( diff --git a/btclib/mnemonic/entropy.py b/btclib/mnemonic/entropy.py index ed9f4c352..8469b1c14 100644 --- a/btclib/mnemonic/entropy.py +++ b/btclib/mnemonic/entropy.py @@ -22,8 +22,8 @@ from hashlib import sha512 from btclib.alias import Octets -from btclib.exceptions import BTClibValueError -from btclib.utils import bytes_from_octets +from btclib.exceptions import BTClibTypeError, BTClibValueError +from btclib.utils import bytes_from_octets, is_integer __all__ = [ "BinStr", @@ -76,9 +76,19 @@ def bin_str_entropy_from_wordlist_indexes(indexes: Sequence[int], base: int) -> Return the raw (i.e. binary 0/1 string) entropy from the provided list of integer indexes into a given language word-list. + + An index the word list has no word for is refused rather than + carried: base-`base` arithmetic accepts any number as a digit, so + 2048 in a 2048-word list is not an error but a carry into the digit + above it -- entropy nothing spells, out of a function whose whole + job is to say what a mnemonic means. """ entropy = 0 for index in indexes: + if not is_integer(index): + raise BTClibTypeError(f"invalid index type: {type(index).__name__}") + if not 0 <= index < base: + raise BTClibValueError(f"invalid index: {index}, not in [0, {base})") entropy = entropy * base + index binentropy = f"{entropy:b}" diff --git a/btclib/number_theory.py b/btclib/number_theory.py index a11402063..d4f833ee0 100644 --- a/btclib/number_theory.py +++ b/btclib/number_theory.py @@ -17,8 +17,8 @@ from __future__ import annotations -from btclib.exceptions import BTClibValueError -from btclib.utils import hex_string +from btclib.exceptions import BTClibTypeError, BTClibValueError +from btclib.utils import hex_string, is_integer __all__ = [ "legendre_symbol", @@ -29,12 +29,49 @@ ] +def _assert_valid_operand(a: int) -> None: + """Refuse an operand that is not an integer, a bool not being one. + + A float goes through every function here without complaint -- + `//`, `%` and `*` are all defined for it -- and comes back out of a + signature that says `int`: `mod_inv(3.0, 7)` answers `5.0`, which is + not a residue and not an error either. `var_int.serialize` checks the + same way and its docstring says why a bool is excluded. + """ + if not is_integer(a): + raise BTClibTypeError(f"not an integer: {a!r}") + + +def _assert_valid_modulus(m: int) -> None: + """Refuse a modulus nothing is a residue of. + + Positive, not merely non-zero: zero is the `ZeroDivisionError` of + `a %= m` and the `ValueError` `pow` raises for a third argument of + zero, neither of which a caller writing `except BTClibValueError` + catches, and a negative modulus would answer with a negative residue + class that no caller of this module has a use for. + """ + _assert_valid_operand(m) + if m < 1: + raise BTClibValueError(f"non-positive modulus: {m}") + + +# every public function here checks its own arguments, rather than five +# private twins doing the work unchecked for the ones that call each +# other: a pair of isinstance calls is a fraction of a percent of the +# arithmetic it stands in front of, an inverse modulo a 256-bit prime +# being an extended Euclid, so the re-checking mod_sqrt does through +# tonelli and legendre_symbol costs less than five more names would + + def xgcd(a: int, b: int) -> tuple[int, int, int]: """Return (g, x, y) such that a*x + b*y = g = gcd(x, y). based on Extended Euclidean Algorithm, see https://en.wikibooks.org/wiki/Algorithm_Implementation/Mathematics/Extended_Euclidean_algorithm """ + _assert_valid_operand(a) + _assert_valid_operand(b) x0, x1, y0, y1 = 0, 1, 1, 0 while a != 0: q, b, a = b // a, a, b % a @@ -51,6 +88,8 @@ def mod_inv(a: int, m: int) -> int: Based on Extended Euclidean Algorithm, see: - https://en.wikibooks.org/wiki/Algorithm_Implementation/Mathematics/Extended_Euclidean_algorithm """ + _assert_valid_operand(a) + _assert_valid_modulus(m) a %= m g, x, _ = xgcd(a, m) if g == 1: @@ -70,6 +109,8 @@ def legendre_symbol(a: int, p: int) -> int: https://codereview.stackexchange.com/questions/43210/tonelli-shanks-algorithm-implementation-of-prime-modular-square-root/43267 """ + _assert_valid_operand(a) + _assert_valid_modulus(p) ls = pow(a, p >> 1, p) return -1 if ls == p - 1 else ls @@ -87,6 +128,8 @@ def mod_sqrt(a: int, p: int) -> int: https://codereview.stackexchange.com/questions/43210/tonelli-shanks-algorithm-implementation-of-prime-modular-square-root/43267 """ + _assert_valid_operand(a) + _assert_valid_modulus(p) a %= p if p % 4 == 3: # secp256k1 case @@ -118,6 +161,8 @@ def tonelli(a: int, p: int) -> int: https://codereview.stackexchange.com/questions/43210/tonelli-shanks-algorithm-implementation-of-prime-modular-square-root/43267 """ + _assert_valid_operand(a) + _assert_valid_modulus(p) a %= p if a == 0 or p == 2: return a diff --git a/btclib/psbt/psbt.py b/btclib/psbt/psbt.py index c48ad8ac0..49268662c 100644 --- a/btclib/psbt/psbt.py +++ b/btclib/psbt/psbt.py @@ -982,7 +982,13 @@ def weight_estimate(self, sizer: SolutionSizer | None = None) -> int: refuses to estimate because what they will push is knowledge only the caller has -- a script of no standard type, a taproot script path. Without one this is that property exactly. + + Validated first, as every other method that reads this psbt's + data is: an estimate off an incoherent psbt is a number, and a + number is what a caller sizes a fee with. """ + self.assert_valid() + vin: list[TxIn] = [] # read once: the transaction is computed at every access, being # the psbt's fields put together rather than a field of its own diff --git a/btclib/script/sig_hash.py b/btclib/script/sig_hash.py index 7f9f9f31a..1297e1720 100644 --- a/btclib/script/sig_hash.py +++ b/btclib/script/sig_hash.py @@ -19,7 +19,7 @@ from btclib import var_bytes from btclib.alias import Octets -from btclib.exceptions import BTClibValueError +from btclib.exceptions import BTClibTypeError, BTClibValueError from btclib.hashes import hash160, hash256, sha256, tagged_hash from btclib.script.script import ( BYTE_FROM_OP_CODE_NAME, @@ -37,7 +37,7 @@ type_and_payload, ) from btclib.tx import Tx, TxIn, TxOut -from btclib.utils import bytes_from_octets +from btclib.utils import bytes_from_octets, is_integer __all__ = [ "ALL", @@ -311,6 +311,22 @@ def _assert_valid_prevouts(prevouts: list[TxOut]) -> None: prevout.script_pub_key.assert_valid() +def _assert_valid_vin_i(tx: Tx, vin_i: int) -> None: + """Refuse an index naming no input of the transaction being signed. + + Every sig_hash here is the hash of *one* input's spend, so an index + outside the vin names nothing to sign. Left to the list it would be + an `IndexError` on the paths that dereference it -- a `LookupError`, + which no `except BTClibValueError` catches -- and on the paths that + only write the index into the preimage it would be no error at all, + just a 32-byte hash of a transaction position that does not exist. + """ + if not is_integer(vin_i): + raise BTClibTypeError(f"invalid input index type: {type(vin_i).__name__}") + if not 0 <= vin_i < len(tx.vin): + raise BTClibValueError(f"invalid input index: {vin_i}") + + def legacy(script_code: Octets, tx: Tx, vin_i: int, hash_type: int) -> bytes: """Return the pre-segwit hash one input's signature commits to. @@ -582,6 +598,7 @@ def taproot( output is an error here, per BIP341, where legacy keeps the bug. """ _assert_valid_prevouts(prevouts) + _assert_valid_vin_i(transaction, input_index) if hashtype not in SIG_HASH_TYPES: raise BTClibValueError(f"Unknown hash type: {hashtype}") diff --git a/btclib/script/taproot.py b/btclib/script/taproot.py index 1048580bb..4453e0d98 100644 --- a/btclib/script/taproot.py +++ b/btclib/script/taproot.py @@ -29,7 +29,7 @@ ) from btclib.curves import bytes_from_prv_key_int, mult, secp256k1 from btclib.curves.curve import _libsecp256k1_applicable, _y_even -from btclib.exceptions import BTClibValueError +from btclib.exceptions import BTClibTypeError, BTClibValueError from btclib.hashes import tagged_hash from btclib.script.limits import MAX_SCRIPT_ELEMENT_SIZE from btclib.script.op_codes_tapscript import ( @@ -40,7 +40,7 @@ from btclib.script.script import _serialize_bytes_command, _serialize_int_command from btclib.to_prv_key import PrvKey, int_from_prv_key from btclib.to_pub_key import Key, pub_keyinfo_from_key -from btclib.utils import bytes_from_octets, bytesio_from_binarydata +from btclib.utils import bytes_from_octets, bytesio_from_binarydata, is_integer __all__ = [ "MAX_TREE_DEPTH", @@ -370,6 +370,10 @@ def input_script_sig( them; the control block is BIP341's -- parity bit plus leaf version, then the x-only internal key, then the merkle path -- and a missing internal key is the unspendable point output_pubkey uses. + + In tree order and counting from zero: Python would read -1 as the + last leaf and hand back a control block that proves it, so a leaf + named from the wrong end is refused rather than answered. """ parity_bit = output_pubkey(internal_pubkey, script_tree)[1] if internal_pubkey: @@ -377,7 +381,12 @@ def input_script_sig( else: h_str = "50929b74c1a04954b78b4b6035e97a5e078a5a0f28ec96d547bfee9ace803ac0" pub_key_bytes = bytes.fromhex(h_str) - (leaf_version, script), path = tree_helper(script_tree)[0][script_num] + leaves = tree_helper(script_tree)[0] + if not is_integer(script_num): + raise BTClibTypeError(f"invalid leaf index type: {type(script_num).__name__}") + if not 0 <= script_num < len(leaves): + raise BTClibValueError(f"invalid leaf index: {script_num}") + (leaf_version, script), path = leaves[script_num] control = (parity_bit + leaf_version).to_bytes(1, "big") control += pub_key_bytes control += path @@ -439,12 +448,18 @@ def check_output_pubkey(q: Octets, script: Octets, control: Octets) -> bool: return Q[0] == int.from_bytes(q, "big") and control[0] & 1 == Q[1] % 2 -def assert_valid_control_block(control_block: bytes) -> None: +def assert_valid_control_block(control_block: Octets) -> None: """Refuse a control block whose size no leaf depth can produce. Size only, and only its residue: one leading byte plus a multiple of 32, which BIP341's 33 + 32m sizes all satisfy. Proving the block against an output key is check_output_pubkey's. + + The octets first, as check_output_pubkey takes them: `len` of the + text spelling counts characters, so "e" * 33 -- 33 characters and + the 66 octets of a hex string -- was a size this accepted, and + "é" * 33, 33 characters and 66 octets of UTF-8, was a size it + accepted for no reason at all. """ - if (len(control_block) - 1) % 32 != 0: + if (len(bytes_from_octets(control_block)) - 1) % 32 != 0: raise BTClibValueError("invalid control block size") diff --git a/btclib/utils.py b/btclib/utils.py index 77c6a44a3..b32f72929 100644 --- a/btclib/utils.py +++ b/btclib/utils.py @@ -214,11 +214,28 @@ def int_from_json_number(value: Any, what: str) -> int: schema mistake would become a version, a depth or an index instead of an error beside the input that caused it. + A *whole* number: 1.0 is the json spelling of 1 and coerces, 1.5 is + the spelling of nothing this library has a field for, and `int` + truncates it to 1 rather than refusing -- silently, and to a number + the caller did write, which is what makes it worse than a type error. + `float.is_integer()` asks that of the value, `nan` and `inf` being no + more whole than 1.5 is. + `is_integer` is the same decision where there is nothing to coerce. """ if isinstance(value, bool): raise BTClibTypeError(f"invalid {what} type: {type(value).__name__}") - return int(value) + if isinstance(value, float) and not value.is_integer(): + raise BTClibValueError(f"invalid {what}: {value}") + try: + return int(value) + # what is left is anything at all, this taking Any: a str that is no + # number, a None, an object. Neither error is btclib's as it stands, + # and `except ValueError` is what a caller of a json reader writes + except TypeError as e: + raise BTClibTypeError(f"invalid {what} type: {type(value).__name__}") from e + except ValueError as e: + raise BTClibValueError(f"invalid {what}: {value!r}") from e def int_from_bits(octets: Octets, nlen: int) -> int: diff --git a/tests/bech32_test.py b/tests/bech32_test.py index 0b3a660ae..115d1a423 100644 --- a/tests/bech32_test.py +++ b/tests/bech32_test.py @@ -47,7 +47,7 @@ decode, encode, ) -from btclib.exceptions import BTClibValueError +from btclib.exceptions import BTClibTypeError, BTClibValueError def test_the_tap_table_holds_the_taps_it_replaces() -> None: @@ -224,3 +224,28 @@ def test_a_changed_character_fails_the_checksum( return with pytest.raises(BTClibValueError, match="invalid checksum: "): decode(f"{hrp}1{mutated}") + + +def test_a_value_that_is_no_5_bit_digit_is_refused() -> None: + """A negative digit wrapped around the alphabet into another address. + + `_ALPHABET[d]` is a plain string index, so Python read -1 as the + last character and -32 as the first: `encode` answered a bech32 + string, correctly checksummed and decodable, spelling a payload + nobody asked for. A digit above 31 raised `IndexError` instead, + which is a `LookupError` and outside every `except BTClibValueError` + written against this library, and a float raised `TypeError`. + """ + valid = [0, 1, 30, 31] + assert decode(encode("bc", valid)) == ("bc", valid) + + for out_of_range in (-1, -32, 32, 2**32): + with pytest.raises(BTClibValueError, match="invalid 5-bit value: "): + encode("bc", [*valid, out_of_range]) + for not_a_digit in (1.0, "1", None, True): + with pytest.raises(BTClibTypeError, match="invalid 5-bit value type: "): + encode("bc", [*valid, not_a_digit]) # type: ignore[list-item] + + # the boundaries themselves: `< 32` weakened to `<= 32`, or `0 <=` + # to `-1 <=`, would accept a digit the alphabet has no character for + assert encode("bc", [31]) != encode("bc", [0]) diff --git a/tests/bip32/bip32_test.py b/tests/bip32/bip32_test.py index a55326b0f..cec3ed6f4 100644 --- a/tests/bip32/bip32_test.py +++ b/tests/bip32/bip32_test.py @@ -464,8 +464,12 @@ def test_derive_exceptions() -> None: with pytest.raises(BTClibValueError, match="index are not a multiple of 4-bytes: "): derive(xprv, b"\x00" * 5) - for index in (2**32, 0x8000000000): - with pytest.raises(OverflowError, match="int too big to convert"): + # an index no path step can hold, refused by the reader rather than + # by the `to_bytes(4, signed=False)` that used to answer it with an + # OverflowError -- an ArithmeticError, outside every `except + # ValueError` a caller of this library writes + for index in (2**32, 0x8000000000, -1): + with pytest.raises(BTClibValueError, match="invalid index: "): derive(xprv, index) # the boundary itself, not only one past it: `> 255` weakened to @@ -1039,6 +1043,31 @@ def test_pub_key_derivation_tweaks_are_32_bytes_each() -> None: assert all(len(tweak) == 32 for tweak in tweaks) +def test_a_path_of_no_steps_still_looks_at_the_public_key() -> None: + """[] is the right answer for an empty path, and not for a non-point. + + `if indexes:` around the whole body made the empty path the one + spelling of this call that validated nothing: 33 bytes that are no + public key -- a prefix of 0x00, an x above p, x = 0 -- all came back + as `[]`, the answer a caller reads as "derivation succeeded, nothing + to apply". + """ + rootxprv = "xprv9s21ZrQH143K3QTDL4LXw2F7HEK3wJUD2nW2nRk4stbPy6cq3jPPqjiChkVvvNKmPGJxWUtg6LnF5kejMRNNU3TGtRBeJgk33yuGBxrMPHi" + xpub = BIP32KeyData.b58decode(xpub_from_xprv(rootxprv)) + + assert pub_key_derivation_tweaks(xpub.key, xpub.chain_code, "m") == [] + + for not_a_point in ( + b"\x02" + b"\xff" * 32, # x above the field prime + b"\x02" + b"\x00" * 32, # x = 0 + b"\x05" + b"\x01" * 32, # no such prefix + b"\x00" + b"\x01" * 32, # the xprv prefix, not a public key + ): + for der_path in ("m", "m/1/2"): + with pytest.raises(BTClibValueError, match="invalid public key: "): + pub_key_derivation_tweaks(not_a_point, xpub.chain_code, der_path) + + def test_derive_with_a_forced_version() -> None: """A forced version must be the same kind, private or public, as the key. diff --git a/tests/bip32/der_path_test.py b/tests/bip32/der_path_test.py index b2d32a4ea..62501f0ca 100644 --- a/tests/bip32/der_path_test.py +++ b/tests/bip32/der_path_test.py @@ -228,3 +228,33 @@ def test_an_iterable_of_indexes_is_checked_element_by_element() -> None: assert hardenings_from_der_path("0h/1", bip380_enforced=True) == ["h", ""] with pytest.raises(BTClibValueError, match="invalid derivation index: "): hardenings_from_der_path("0H/1", bip380_enforced=True) + + +def test_an_index_outside_the_32_bits_a_step_holds_is_refused() -> None: + """The bound the text spelling enforces, on the spellings that are not. + + A step is one of 2**32 indexes however the path was written, and + only the string spelling said so: it parses each step against + `0 <= index < 0x80000000` and adds the offset for a hardening + symbol, while the 4-byte spelling cannot go out of range by + construction. The int and the iterable had nothing, so + `indexes_from_der_path([-5])` answered `[-5]` -- the malformed index + handed straight back, no exception, and an OverflowError far away in + whatever went on to serialize it. + """ + for out_of_range in (-1, 2**32, 2**40): + with pytest.raises(BTClibValueError, match="invalid index: "): + indexes_from_der_path(out_of_range) + with pytest.raises(BTClibValueError, match="invalid index: "): + indexes_from_der_path([0, out_of_range]) + with pytest.raises(BTClibValueError, match="invalid index: "): + bytes_from_der_path([out_of_range]) + + # the boundaries themselves: `<= 0xffffffff` weakened to `<`, or + # `0 <=` to `0 <`, would refuse a step every hardened path holds + assert indexes_from_der_path([0, 0xFFFFFFFF]) == [0, 0xFFFFFFFF] + assert indexes_from_der_path(0xFFFFFFFF) == [0xFFFFFFFF] + + # the two spellings that were already bounded, unchanged + assert indexes_from_der_path("m/0h") == [0x80000000] + assert indexes_from_der_path(bytes.fromhex("ffffffff")) == [0xFFFFFFFF] diff --git a/tests/bip32/key_origin_test.py b/tests/bip32/key_origin_test.py index c494f0bed..7890ad717 100644 --- a/tests/bip32/key_origin_test.py +++ b/tests/bip32/key_origin_test.py @@ -5,6 +5,7 @@ """Tests for the `btclib.bip32.key_origin` module.""" import dataclasses +from typing import cast import pytest @@ -37,14 +38,25 @@ def test_bip32_key_origin() -> None: # already carries a shorter version of assert len(BIP32KeyOrigin("deadbeef", [0] * 255)) == 255 - with pytest.raises(BTClibValueError, match="invalid der_path element"): + # the path reader refuses these before the constructor stores them, + # so this is its message and not assert_valid's + with pytest.raises(BTClibValueError, match="invalid index: "): BIP32KeyOrigin("deadbeef", [0xFFFFFFFF + 1]) - with pytest.raises(BTClibValueError, match="invalid der_path element"): + with pytest.raises(BTClibValueError, match="invalid index: "): BIP32KeyOrigin("deadbeef", [-1]) # the upper boundary itself: `<= 0xffffffff` weakened to `< # 0xffffffff` or to `<= 0xfffffffe` would refuse it assert BIP32KeyOrigin("deadbeef", [0xFFFFFFFF]).der_path == [0xFFFFFFFF] + # and assert_valid still asks. The field is annotated Sequence[int] + # and holds the list `indexes_from_der_path` built, so the dataclass + # being frozen stops a rebinding and not an append: the constructor + # is not the only way an out-of-range index reaches the field + key_origin = BIP32KeyOrigin("deadbeef", [0]) + cast("list[int]", key_origin.der_path).append(0xFFFFFFFF + 1) + with pytest.raises(BTClibValueError, match="invalid der_path element"): + key_origin.assert_valid() + description = master_fingerprint = "deadbeef" key_origin = BIP32KeyOrigin.from_description(description) assert len(key_origin) == 0 diff --git a/tests/descriptors/miniscript_test.py b/tests/descriptors/miniscript_test.py index 56f0ed06b..5aaeefd88 100644 --- a/tests/descriptors/miniscript_test.py +++ b/tests/descriptors/miniscript_test.py @@ -1194,6 +1194,25 @@ def test_the_solver_answers_for_what_is_its_business_and_no_more() -> None: assert miniscript_solver(psbt, 0) is None +def test_the_solver_names_an_input_that_exists() -> None: + """A negative index solved the input at the other end, and said nothing. + + `psbt.inputs[vin_i]` is a list index, so -1 was the last input: the + solver read another input's witness script and preimages and + answered a witness for it, which the caller then attaches to the + input it did ask about. Its siblings `update_psbt_input` and + `update_psbt_output` carry this guard already, with the comment + saying why. + """ + node = parse(f"and_v(v:pk({KEYS[0]}),older(36))") + psbt, _ = signed_psbt(node, [KEYS[0]], 499999999, 36) + assert miniscript_solver(psbt, 0) is not None + + for out_of_range in (-1, 1, 99): + with pytest.raises(BTClibValueError, match="invalid input index: "): + miniscript_solver(psbt, out_of_range) + + def test_the_solver_reads_a_key_the_input_knows_by_its_hash() -> None: """Recognize a pkh() fragment, whose script holds no key. diff --git a/tests/fetch/fetcher_test.py b/tests/fetch/fetcher_test.py index 5af7ea890..dc943eb99 100644 --- a/tests/fetch/fetcher_test.py +++ b/tests/fetch/fetcher_test.py @@ -8,7 +8,12 @@ import pytest -from btclib.exceptions import BTClibRuntimeError, BTClibValueError, FetchError +from btclib.exceptions import ( + BTClibRuntimeError, + BTClibTypeError, + BTClibValueError, + FetchError, +) from btclib.fetch.fetcher import ( Fetcher, fetch_errors, @@ -78,6 +83,26 @@ def test_tx_for_network_leaves_mainnet_alone() -> None: """Return the very same object for mainnet, not a relabelled copy.""" tx = Tx.parse(RAW) assert tx_for_network(tx, "mainnet") is tx + # and for every other spelling of it, the name being resolved and not + # compared: " MainNet " used to fall through to the relabelling branch + assert tx_for_network(tx, " MainNet ") is tx + + +def test_tx_for_network_refuses_a_network_no_table_has() -> None: + """The label is written with check_validity=False, so nothing caught it. + + Every `ScriptPubKey`, `TxOut` and `Tx` built here is built unchecked + -- deliberately, the relabelling touching nothing else -- so a + network name that names nothing was baked into the transaction + handed back, to surface as a `KeyError` from whatever went on to + render an address, a long way from the call that caused it. + """ + tx = Tx.parse(RAW) + for unknown in ("mainet", "", "bitcoin"): + with pytest.raises(BTClibValueError, match="unknown network: "): + tx_for_network(tx, unknown) + with pytest.raises(BTClibTypeError, match="not a network name: "): + tx_for_network(tx, None) # type: ignore[arg-type] def test_tx_for_network_labels_the_outputs_and_touches_nothing_else() -> None: diff --git a/tests/integer_policy_test.py b/tests/integer_policy_test.py index 357510a61..8ebcd7b15 100644 --- a/tests/integer_policy_test.py +++ b/tests/integer_policy_test.py @@ -21,7 +21,8 @@ import pytest -from btclib import base58, var_int +from btclib import base58, bech32, var_int +from btclib.alias import TaprootScriptTree from btclib.amount import valid_sats_amount from btclib.bip32 import BIP32KeyData from btclib.bip32.der_path import ( @@ -34,12 +35,19 @@ from btclib.block.block_context import BlockContext from btclib.exceptions import BTClibTypeError from btclib.fee import FeeRate, fee_from_vsize +from btclib.mnemonic.entropy import bin_str_entropy_from_wordlist_indexes +from btclib.number_theory import mod_inv +from btclib.script import input_script_sig, sig_hash from btclib.tx import OutPoint, Tx, TxIn, TxOut from btclib.utils import bytes_from_octets, is_integer _TX_ID = "01" * 32 _RATE = FeeRate(sats_per_kvbyte=1000) _NOW = datetime(2026, 8, 4, tzinfo=timezone.utc) +# a one-leaf tree and the prevout of the one input `_tx` builds: what the +# two index parameters below have to be handed something valid to index +_SCRIPT_TREE: TaprootScriptTree = [(0xC0, ["OP_1"])] +_PREVOUTS = [TxOut(1, b"")] def _tx(version: Any = 1, lock_time: Any = 0) -> Tx: @@ -104,6 +112,15 @@ def _header(version: Any = 1, nonce: Any = 1) -> BlockHeader: ("base58 output size", lambda v: base58.decode(base58.encode(b"x"), v)), ("var_int", var_int.serialize), ("var_int max_size", lambda v: var_int.parse(b"\x01", max_size=v)), + ("bech32 5-bit value", lambda v: bech32.encode("bc", [v])), + ("word-list index", lambda v: bin_str_entropy_from_wordlist_indexes([v], 2048)), + ("modular operand", lambda v: mod_inv(v, 7)), + ("modulus", lambda v: mod_inv(3, v)), + ("taproot leaf index", lambda v: input_script_sig(None, _SCRIPT_TREE, v)), + ( + "sig_hash input index", + lambda v: sig_hash.taproot(_tx(), v, _PREVOUTS, 1, 0, b"", b""), + ), ] _IDS = [case[0] for case in _CASES] @@ -152,6 +169,11 @@ def test_the_integers_a_bool_refusal_must_not_take_with_it() -> None: assert base58.decode(base58.encode(b"x"), 1) == b"x" assert var_int.serialize(1) == b"\x01" assert var_int.parse(b"\x01", max_size=1) == 1 + assert bech32.encode("bc", [1]) == b"bc1pdg93mv" + assert bin_str_entropy_from_wordlist_indexes([1], 2048) == "00000000001" + assert mod_inv(3, 7) == 5 + assert input_script_sig(None, _SCRIPT_TREE, 0)[0] == ["OP_1"] + assert len(sig_hash.taproot(_tx(), 0, _PREVOUTS, 1, 0, b"", b"")) == 32 # the str and bytes spellings of a path are untouched by any of it assert indexes_from_der_path("m/44h/0h") == [2147483692, 2147483648] diff --git a/tests/mnemonic/entropy_test.py b/tests/mnemonic/entropy_test.py index 3e088e1bf..9c9052d2f 100644 --- a/tests/mnemonic/entropy_test.py +++ b/tests/mnemonic/entropy_test.py @@ -10,7 +10,7 @@ import pytest -from btclib.exceptions import BTClibValueError +from btclib.exceptions import BTClibTypeError, BTClibValueError from btclib.mnemonic import ( bin_str_entropy_from_bytes, bin_str_entropy_from_entropy, @@ -46,6 +46,31 @@ def test_indexes() -> None: assert indexes == expected +def test_an_index_no_word_answers_to_is_refused() -> None: + """Out of range is a carry in base-`base`, not an error, and so was wrong. + + `entropy * base + index` accepts any number as a digit: 2048 in a + 2048-word list is the same accumulation as a 1 carried into the + digit above it, and a negative index subtracts. What came back was + entropy no mnemonic spells, from the function whose whole job is to + say what one means -- and it came back with no exception at all. + """ + good = [0, 1, 2046, 2047] + assert int(bin_str_entropy_from_wordlist_indexes(good, 2048), 2) == ( + ((1 * 2048) + 2046) * 2048 + 2047 + ) + + for out_of_range in (-1, 2048, 2**32): + with pytest.raises(BTClibValueError, match="invalid index: "): + bin_str_entropy_from_wordlist_indexes([*good, out_of_range], 2048) + # the same index, in the base that does have a word for it + assert bin_str_entropy_from_wordlist_indexes([2048], 4096) + + for not_an_index in (1.0, "1", True): + with pytest.raises(BTClibTypeError, match="invalid index type: "): + bin_str_entropy_from_wordlist_indexes([not_an_index], 2048) # type: ignore[list-item] + + def test_indexes_round_trip_a_base_that_is_not_a_power_of_two() -> None: """2048 is a power of two, where `+`, `|` and `^` all agree; 1626 is not. diff --git a/tests/number_theory_test.py b/tests/number_theory_test.py index fa851f5b6..3c2dbb0f8 100644 --- a/tests/number_theory_test.py +++ b/tests/number_theory_test.py @@ -10,7 +10,7 @@ from hypothesis import given from hypothesis import strategies as st -from btclib.exceptions import BTClibValueError +from btclib.exceptions import BTClibTypeError, BTClibValueError from btclib.number_theory import legendre_symbol, mod_inv, mod_sqrt, tonelli, xgcd primes = [ @@ -56,6 +56,45 @@ ] +def test_a_float_is_no_operand_and_zero_is_no_modulus() -> None: + """Modular arithmetic over what is not an integer answered anyway. + + Python defines `//`, `%` and `*` for a float, so every function here + ran to completion on one and returned it: `mod_inv(3.0, 7)` answered + `5.0` out of a signature that says `int`, and `xgcd(3.0, 7)` a + triple of floats -- no exception, and a residue that is not one. + `legendre_symbol` and its two callers reached `pow`'s own TypeError + instead, which is no better for a caller filtering bad input. + + A modulus of zero is the other half: the `ZeroDivisionError` of + `a %= m` and the `ValueError` `pow` raises for a third argument of + zero. The first is an `ArithmeticError`, so `except ValueError` does + not catch it. + """ + for a, m in ((3.0, 7), (3, 7.0), ("3", 7), (3, None)): + with pytest.raises(BTClibTypeError, match="not an integer: "): + xgcd(a, m) # type: ignore[arg-type] + for call in (mod_inv, legendre_symbol, mod_sqrt, tonelli): + with pytest.raises(BTClibTypeError, match="not an integer: "): + call(a, m) # type: ignore[arg-type] + + # a bool is not a number either, `isinstance(True, int)` being what + # would otherwise make it the modulus one + for value in (True, False): + with pytest.raises(BTClibTypeError, match="not an integer: "): + mod_inv(value, 7) + with pytest.raises(BTClibTypeError, match="not an integer: "): + mod_inv(3, value) + + for m in (0, -7): + for call in (mod_inv, legendre_symbol, mod_sqrt, tonelli): + with pytest.raises(BTClibValueError, match="non-positive modulus: "): + call(3, m) + # xgcd takes no modulus: zero is a legitimate operand there, whose + # greatest common divisor with three is three + assert xgcd(3, 0)[0] == 3 + + def test_mod_inv_prime() -> None: """Verify the inverse mod a prime, and refuse the zero residue.""" for p in primes: diff --git a/tests/psbt/psbt_size_test.py b/tests/psbt/psbt_size_test.py index 831aad139..48fd3063e 100644 --- a/tests/psbt/psbt_size_test.py +++ b/tests/psbt/psbt_size_test.py @@ -246,6 +246,28 @@ def test_the_bip174_example_is_bounded_by_its_signed_transaction() -> None: assert psbt.estimated_weight - signed.weight == 6 +def test_an_incoherent_psbt_is_not_estimated() -> None: + """A weight is a number, and a number is what a fee is computed from. + + Every other public method of `Psbt` that reads this psbt's data + validates it first; this one did not, so a psbt BIP174 calls invalid + -- here a v0 carrying the v2-only PSBT_GLOBAL_TX_MODIFIABLE -- came + back with an estimate rather than with the refusal `assert_valid` + was there to give. + """ + psbt = unsigned(Psbt.b64decode(BIP174_SIGNED_PSBT)) + assert psbt.weight_estimate() > 0 + + psbt.tx_modifiable = 1 + err_msg = "PSBT_GLOBAL_TX_MODIFIABLE is not allowed in a v0 psbt" + with pytest.raises(BTClibValueError, match=err_msg): + psbt.weight_estimate() + with pytest.raises(BTClibValueError, match=err_msg): + _ = psbt.estimated_weight + with pytest.raises(BTClibValueError, match=err_msg): + _ = psbt.estimated_vsize + + def test_a_finalized_input_is_not_estimated() -> None: """What the Finalizer produced is what the transaction will carry.""" psbt = Psbt.b64decode(BIP174_FINALIZED_PSBT) diff --git a/tests/script/sig_hash_taproot_test.py b/tests/script/sig_hash_taproot_test.py index b85f4bd8a..3ba7f61ba 100644 --- a/tests/script/sig_hash_taproot_test.py +++ b/tests/script/sig_hash_taproot_test.py @@ -23,7 +23,11 @@ from btclib.alias import ScriptList, TaprootScriptTree from btclib.ecc import ssa -from btclib.exceptions import BTClibRuntimeError, BTClibValueError +from btclib.exceptions import ( + BTClibRuntimeError, + BTClibTypeError, + BTClibValueError, +) from btclib.hashes import hash160, sha256 from btclib.script import ( ScriptPubKey, @@ -623,3 +627,41 @@ def test_the_spent_amounts_are_signed_camounts() -> None: assert sig_hash.taproot( tx, 0, [prevout], sig_hash.ANYONECANPAY | sig_hash.ALL, 0, b"", b"" ) + + +def test_an_input_index_outside_the_vin_is_refused() -> None: + """Two indexes of no input gave two hashes, both accepted as answers. + + BIP341's SigMsg commits to the index itself, and outside the + ANYONECANPAY branch that is all it does with it: nothing dereferences + `transaction.vin[input_index]`, so an index past the end was written + into the preimage and hashed. 99 and 100 on a two-input transaction + therefore produced two *different* 32-byte hashes, each a valid + answer to a question about an input that does not exist. The bound + was checked in the SIGHASH_SINGLE branch alone, against the vout. + """ + utxo = TxOut( + 100000000, + serialize( + ["OP_1", "cc71eb30d653c0c3163990c47b976f3fb3f37cccdcbedb169a1dfef58bbfbfaf"] + ), + ) + vin = [ + TxIn(OutPoint("01" * 32, 0), "", 1, Witness(["00" * 64])), + TxIn(OutPoint("02" * 32, 1), "", 1, Witness(["00" * 64])), + ] + tx = Tx(vin=vin, vout=[TxOut(100000000, ""), TxOut(1, "")]) + prevouts = [utxo, utxo] + + def sig_hash_of(input_index: int) -> bytes: + return sig_hash.taproot(tx, input_index, prevouts, 0x01, 0, b"", b"") + + assert len(sig_hash_of(0)) == 32 + assert sig_hash_of(0) != sig_hash_of(1) + + for out_of_range in (-1, 2, 99, 100): + with pytest.raises(BTClibValueError, match="invalid input index: "): + sig_hash_of(out_of_range) + for not_an_index in (1.0, "0", True): + with pytest.raises(BTClibTypeError, match="invalid input index type: "): + sig_hash_of(not_an_index) # type: ignore[arg-type] diff --git a/tests/script/taproot_test.py b/tests/script/taproot_test.py index 4252e3e16..c24347aa3 100644 --- a/tests/script/taproot_test.py +++ b/tests/script/taproot_test.py @@ -19,7 +19,7 @@ from btclib import b32 from btclib.alias import ScriptList from btclib.curves import bytes_from_point, curve_group, mult -from btclib.exceptions import BTClibValueError +from btclib.exceptions import BTClibTypeError, BTClibValueError from btclib.script import ( TaprootScriptTree, Witness, @@ -34,7 +34,12 @@ type_and_payload, ) from btclib.script.limits import MAX_SCRIPT_ELEMENT_SIZE -from btclib.script.taproot import parse, serialize, tree_helper +from btclib.script.taproot import ( + assert_valid_control_block, + parse, + serialize, + tree_helper, +) from btclib.tx import TxOut from tests import load, vector_id from tests.curves.curve_test import low_card_curves @@ -245,6 +250,55 @@ def test_invalid_control_block() -> None: check_output_pubkey(b"\x00" * 32, b"\x00", b"\x00" * 100) +def test_a_control_block_size_is_octets_and_not_characters() -> None: + """`len` of the text spelling counted characters, and let two sizes in. + + The residue check is `(len - 1) % 32`, and it ran on whatever it was + handed: `"é" * 33` is 33 characters and 66 octets of UTF-8, and + passed as a control block size; 66 characters of hex are 33 octets + and were refused. The coercion `check_output_pubkey` does on the + same argument is what makes the number octets. + """ + # 33 octets, one leading byte and a merkle path of none, in both + # spellings of the same block + assert_valid_control_block(b"\x00" * 33) + assert_valid_control_block("00" * 33) + # 65: one byte and one 32-byte merkle path + assert_valid_control_block(b"\x00" * 65) + + err_msg = "invalid control block size" + for wrong_size in (b"\x00" * 34, "00" * 34, b"", b"\x00" * 66): + with pytest.raises(BTClibValueError, match=err_msg): + assert_valid_control_block(wrong_size) + + # a str that is no hex string reaches the size check no longer; the + # class is `bytes_from_octets`'s to tighten, which issue 744's last + # slice is about + for not_octets in ("é" * 33, "a" * 33): + with pytest.raises(ValueError, match="fromhex"): + assert_valid_control_block(not_octets) + + +def test_a_leaf_is_named_from_the_start_of_the_tree() -> None: + """-1 is the last leaf to Python, and was answered as one. + + `tree_helper(script_tree)[0][script_num]` is a list index, so a + negative one selected a leaf from the other end and built a control + block that correctly proves it: the caller asked for a leaf that + does not exist and got a valid script-path spend of another one. + """ + script_tree: TaprootScriptTree = [[(0xC0, ["OP_2"])], [(0xC0, ["OP_3"])]] + assert input_script_sig(None, script_tree, 0)[0] == ["OP_2"] + assert input_script_sig(None, script_tree, 1)[0] == ["OP_3"] + + for out_of_range in (-1, -2, 2, 99): + with pytest.raises(BTClibValueError, match="invalid leaf index: "): + input_script_sig(None, script_tree, out_of_range) + for not_an_index in (1.0, "1", True): + with pytest.raises(BTClibTypeError, match="invalid leaf index type: "): + input_script_sig(None, script_tree, not_an_index) # type: ignore[arg-type] + + def test_unspendable_script() -> None: """Refuse output_pubkey with neither a key nor a script tree.""" err_msg = "missing data" diff --git a/tests/utils_test.py b/tests/utils_test.py index 2ca645f29..cfef09a37 100644 --- a/tests/utils_test.py +++ b/tests/utils_test.py @@ -9,7 +9,7 @@ import pytest -from btclib.exceptions import BTClibValueError +from btclib.exceptions import BTClibTypeError, BTClibValueError from btclib.utils import ( assert_no_trailing, decode_num, @@ -17,6 +17,7 @@ hex_string, int_from_bits, int_from_integer, + int_from_json_number, read_exactly, ) @@ -217,3 +218,35 @@ def test_encode_num_is_bounded_by_the_int64_of_a_script_number() -> None: assert decode_num(encode_num(2**63 - 1)) == 2**63 - 1 assert decode_num(encode_num(-(2**63))) == -(2**63) assert decode_num(bytes.fromhex("00000000000000000010")) == 2**76 + + +def test_a_json_number_is_a_whole_one_or_it_is_an_error() -> None: + """1.0 is the json spelling of 1; 1.5 is the spelling of nothing. + + `from_dict` coerces because a whole number may arrive as a float, + which is what json does to 1 -- but `int(1.5)` is 1, so a fractional + version, depth or index became a number the caller never wrote and + nothing said so. `nan` and `inf` are floats and no more whole than + 1.5 is. + """ + assert int_from_json_number(1, "version") == 1 + assert int_from_json_number(1.0, "version") == 1 + assert int_from_json_number(-1.0, "version") == -1 + assert int_from_json_number("1", "version") == 1 + + for fractional in (1.5, -0.5, float("nan"), float("inf")): + with pytest.raises(BTClibValueError, match="invalid version: "): + int_from_json_number(fractional, "version") + + # a bool decodes out of json's `true` and `int(True)` is 1 + for value in (True, False): + with pytest.raises(BTClibTypeError, match="invalid version type: bool"): + int_from_json_number(value, "version") + + # and what is no number at all, this taking Any: neither error was + # btclib's, and one of them was not even a ValueError + with pytest.raises(BTClibValueError, match="invalid version: "): + int_from_json_number("not a number", "version") + for not_a_number in (None, object(), [1]): + with pytest.raises(BTClibTypeError, match="invalid version type: "): + int_from_json_number(not_a_number, "version")