TxOut.__init__ takes script_pub_key: ScriptPubKey | Octets. The
octets branch is validated — ScriptPubKey(script_bytes) defaults to
check_validity=True — and the object branch is stored as it is.
TxOut.assert_valid then checks only the amount, so nothing ever asks
the script whether it is one:
ScriptPubKey.assert_valid: REFUSED -> unknown network: notanetwork
TxOut(1000, <bad spk>) accepted; network stored = 'notanetwork'
octets branch instead: accepted (valid script, as expected)
The object is ScriptPubKey(b"\x51", "notanetwork", check_validity=False).
It matters more than the one class, because TxOut is what carries a
script into everything else: a Tx's outputs, a Block's transactions,
and PsbtIn.witness_utxo — which is the amount and script a segwit
signature commits to. A TxOut that passes its own assert_valid is
taken as sound by all of them.
This is also the seed of two findings filed separately: the TxOut with
an out-of-range amount that #690 walks through sig_hash, and the psbt
inputs of #692.
The fix
The rule of #684, applied to a constructor rather than a function: the
object branch validates as the octets branch does. One line, and the
same shape the sibling classes already use where they take a pre-built
member.
Worth checking the sibling constructors in the same sweep — Tx,
Block, PsbtIn, PsbtOut all take pre-built members too, and only
TxOut was reproduced here.
Not a violation, and why the fix should say so
OutPoint and TxIn are valid by construction: their fields are widths
the parse enforces, so the semantic check is unreachable by design
rather than skipped. btclib/utils.py documents that rule already.
Whatever lands here should name them as the exceptions they are, rather
than leaving the next reader to work out why two classes in the same
directory were left alone.
TxOut.__init__takesscript_pub_key: ScriptPubKey | Octets. Theoctets branch is validated —
ScriptPubKey(script_bytes)defaults tocheck_validity=True— and the object branch is stored as it is.TxOut.assert_validthen checks only the amount, so nothing ever asksthe script whether it is one:
The object is
ScriptPubKey(b"\x51", "notanetwork", check_validity=False).It matters more than the one class, because
TxOutis what carries ascript into everything else: a
Tx's outputs, aBlock's transactions,and
PsbtIn.witness_utxo— which is the amount and script a segwitsignature commits to. A
TxOutthat passes its ownassert_validistaken as sound by all of them.
This is also the seed of two findings filed separately: the
TxOutwithan out-of-range amount that #690 walks through
sig_hash, and the psbtinputs of #692.
The fix
The rule of #684, applied to a constructor rather than a function: the
object branch validates as the octets branch does. One line, and the
same shape the sibling classes already use where they take a pre-built
member.
Worth checking the sibling constructors in the same sweep —
Tx,Block,PsbtIn,PsbtOutall take pre-built members too, and onlyTxOutwas reproduced here.Not a violation, and why the fix should say so
OutPointandTxInare valid by construction: their fields are widthsthe parse enforces, so the semantic check is unreachable by design
rather than skipped.
btclib/utils.pydocuments that rule already.Whatever lands here should name them as the exceptions they are, rather
than leaving the next reader to work out why two classes in the same
directory were left alone.