`channels_sv2::bip141::try_strip_bip141` needs tighter validation against `coinbase_tx_prefix` and `coinbase_tx_suffix`

[plebhash]

channels_sv2::bip141::try_strip_bip141 is meant to help strip BIP141 segwit marker, flag and witness data from a coinbase tx split across coinbase_tx_prefix and coinbase_tx_suffix of a NewExtendedMiningJob:

stratum/sv2/channels-sv2/src/bip141.rs

Lines 33 to 63 in c1a7991

	pub fn try_strip_bip141(
	coinbase_tx_prefix: &[u8],
	coinbase_tx_suffix: &[u8],
	) -> Result<Option<(Vec<u8>, Vec<u8>)>, StripBip141Error> {
	// https://github.com/bitcoinbook/bitcoinbook/blob/third_edition_github/ch06_transactions.adoc#extended-marker-and-flag
	let has_bip141_marker_and_flag =
	(coinbase_tx_prefix[MARKER_OFFSET] == 0x00) && (coinbase_tx_prefix[FLAG_OFFSET] != 0x00);
	if !has_bip141_marker_and_flag {
	return Ok(None);
	}
	// strip bip141 marker and flag bytes from coinbase_tx_prefix
	let mut coinbase_tx_prefix_stripped_bip141 = coinbase_tx_prefix[0..MARKER_OFFSET].to_vec();
	coinbase_tx_prefix_stripped_bip141
	.extend_from_slice(&coinbase_tx_prefix[MARKER_OFFSET + MARKER_FLAG_LEN..]);
	// strip bip141 witness bytes from coinbase_tx_suffix
	let locktime_position = coinbase_tx_suffix.len() - LOCKTIME_LEN;
	// strip witness count, witness length and witness data
	let mut coinbase_tx_suffix_stripped_bip141 = coinbase_tx_suffix
	[..locktime_position - WITNESS_COUNT_LEN - WITNESS_LEN_LEN - WITNESS_DATA_LEN]
	.to_vec();
	coinbase_tx_suffix_stripped_bip141.extend_from_slice(&coinbase_tx_suffix[locktime_position..]);
	Ok(Some((
	coinbase_tx_prefix_stripped_bip141,
	coinbase_tx_suffix_stripped_bip141,
	)))
	}

the current implementation is too naive, because it uses unchecked indexing and subtraction

• if a coinbase_tx_prefix size violates assumptions around MARKER_OFFSET and FLAG_OFFSET (needs prefix ≥ 6 bytes), we panic on indexing
• if a coinbase_tx_suffix size violates assumptions around LOCKTIME_LEN and WITNESS_COUNT_LEN + WITNESS_LEN_LEN + WITNESS_DATA_LEN (needs suffix ≥ 38 bytes), we panic on subtraction underflow

as a consequence, a malformed NewExtendedMiningJob could cause panics on client apps that rely on this code