Refuse the malformed input twelve places answered anyway

CHANGELOG.md

@@ -1712,6 +1712,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

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)

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
 
 

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
 
 

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

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(

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}"

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

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