PartiallySignedTransaction::SetupFromTx indexes vtxinwit checked
What the developer wrote
Authored by Tom Trevethan
PartiallySignedTransaction::SetupFromTx indexes vtxinwit checked
What changed, and why it matters
This commit hardens how Elements handles 'peg-in' transactions—transfers of assets from a parent blockchain into a sidechain. It adds size checks for witness data fields and wraps decoding in error handling so malformed inputs are rejected cleanly. It also fixes a place where the code could read past the end of an array when setting up a Partially Signed Transaction (PSBT). The changes are defensive: they prevent crashes or undefined behavior from bad transaction data, but they do not by themselves create new asset theft or remote-code-execution paths.
Treat as a security hardening fix and include in the next maintenance release. Review whether any other peg-in parsing sites assume well-formed witness data or index vtxinwit without bounds checks. No immediate emergency response is indicated, but users processing untrusted PSBTs or peg-in proofs should upgrade once a release is available.
Security signals we found
Out-of-bounds array access fixed in PSBT peg-in setup
Missing length validation added for pegin witness stack elements
Exception handling added around deserialization of pegin witness components
Asset-mismatch assertion replaced with conditional validation
Defensive hardening of peg-in transaction parsing
Evidence from the diff
The patch modifies two files. In src/pegins.cpp, DecomposePeginWitness now validates that the script witness stack has exactly six items and that the asset and parent-genesis-hash entries are 32 bytes before parsing. Parsing is moved into a try/catch so malformed serialization no longer propagates exceptions. In src/psbt.cpp, PartiallySignedTransaction::SetupFromTx now checks i < tx.witness.vtxinwit.size() before indexing vtxinwit for a peg-in input, and it only populates peg-in PSBT fields when the decomposed asset matches the consensus pegged asset. Previously the code could index out of bounds and assert on unexpected assets.
Changed components
src/pegins.cppsrc/psbt.cppPartiallySignedTransaction::SetupFromTxDecomposePeginWitnesspeg-in input handlingInspect captured patch +57 / −33
### src/pegins.cpp
@@ -551,40 +551,55 @@ bool DecomposePeginWitness(const CScriptWitness& witness, CAmount& value, CAsset
const auto& stack = witness.stack;
if (stack.size() != 6) return false;
+ if (stack[1].size() != 32) return false; // asset
+ if (stack[2].size() != 32) return false; // parent genesis hash
- DataStream stream{stack[0]};
- stream >> value;
-
- CAsset tmp_asset(stack[1]);
- asset = tmp_asset;
-
- uint256 gh(stack[2]);
- genesis_hash = gh;
-
- CScript s(stack[3].begin(), stack[3].end());
- claim_script = s;
+ CAmount tmp_value{0};
+ CAsset tmp_asset;
+ uint256 tmp_genesis_hash;
+ CScript tmp_claim_script;
+ std::variant<std::monostate, Sidechain::Bitcoin::CTransactionRef, CTransactionRef> tmp_tx;
+ std::variant<std::monostate, Sidechain::Bitcoin::CMerkleBlock, CMerkleBlock> tmp_merkle_block;
- DataStream ss_tx(stack[4]);
- if (Params().GetConsensus().ParentChainHasPow()) {
- Sidechain::Bitcoin::CTransactionRef btc_tx;
- ss_tx >> TX_WITH_WITNESS(btc_tx);
- tx = btc_tx;
- } else {
- CTransactionRef elem_tx;
- ss_tx >> TX_WITH_WITNESS(elem_tx);
- tx = elem_tx;
- }
+ try {
+ DataStream stream{stack[0]};
+ stream >> tmp_value;
+
+ tmp_asset = CAsset(stack[1]);
+ tmp_genesis_hash = uint256(stack[2]);
+ tmp_claim_script = CScript(stack[3].begin(), stack[3].end());
+
+ DataStream ss_tx(stack[4]);
+ if (Params().GetConsensus().ParentChainHasPow()) {
+ Sidechain::Bitcoin::CTransactionRef btc_tx;
+ ss_tx >> TX_WITH_WITNESS(btc_tx);
+ tmp_tx = btc_tx;
+ } else {
+ CTransactionRef elem_tx;
+ ss_tx >> TX_WITH_WITNESS(elem_tx);
+ tmp_tx = elem_tx;
+ }
- DataStream ss_proof(stack[5]);
- if (Params().GetConsensus().ParentChainHasPow()) {
- Sidechain::Bitcoin::CMerkleBlock tx_proof;
- ss_proof >> TX_WITH_WITNESS(tx_proof);
- merkle_block = tx_proof;
- } else {
- CMerkleBlock tx_proof;
- ss_proof >> TX_WITH_WITNESS(tx_proof);
- merkle_block = tx_proof;
+ DataStream ss_proof(stack[5]);
+ if (Params().GetConsensus().ParentChainHasPow()) {
+ Sidechain::Bitcoin::CMerkleBlock tx_proof;
+ ss_proof >> TX_WITH_WITNESS(tx_proof);
+ tmp_merkle_block = tx_proof;
+ } else {
+ CMerkleBlock tx_proof;
+ ss_proof >> TX_WITH_WITNESS(tx_proof);
+ tmp_merkle_block = tx_proof;
+ }
+ } catch (const std::exception&) {
+ // Malformed encoding. Report failure rather than propagating
+ return false;
}
+ value = tmp_value;
+ asset = tmp_asset;
+ genesis_hash = tmp_genesis_hash;
+ claim_script = tmp_claim_script;
+ tx = std::move(tmp_tx);
+ merkle_block = std::move(tmp_merkle_block);
return true;
}
### src/psbt.cpp
@@ -1022,12 +1022,21 @@ void PartiallySignedTransaction::SetupFromTx(const CMutableTransaction& tx)
}
}
// Peg-in things
- if (txin.m_is_pegin) {
+ if (txin.m_is_pegin && i < tx.witness.vtxinwit.size()) {
CAmount peg_in_value;
CAsset asset;
- if (DecomposePeginWitness(tx.witness.vtxinwit[i].m_pegin_witness, peg_in_value, asset, input.m_peg_in_genesis_hash, input.m_peg_in_claim_script, input.m_peg_in_tx, input.m_peg_in_txout_proof)) {
+ uint256 genesis_hash;
+ CScript claim_script;
+ std::variant<std::monostate, Sidechain::Bitcoin::CTransactionRef, CTransactionRef> peg_in_tx;
+ std::variant<std::monostate, Sidechain::Bitcoin::CMerkleBlock, CMerkleBlock> txout_proof;
+ if (DecomposePeginWitness(tx.witness.vtxinwit[i].m_pegin_witness, peg_in_value, asset,
+ genesis_hash, claim_script, peg_in_tx, txout_proof)
+ && asset == Params().GetConsensus().pegged_asset) {
input.m_peg_in_value = peg_in_value;
- assert(asset == Params().GetConsensus().pegged_asset);
+ input.m_peg_in_genesis_hash = genesis_hash;
+ input.m_peg_in_claim_script = claim_script;
+ input.m_peg_in_tx = peg_in_tx;
+ input.m_peg_in_txout_proof = txout_proof;
}
}
}Why this scored 60/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.