Skip to content

Commit 2f1fd85

Browse files
upbqdnconradoplg
andauthored
fix(consensus)!: reject non-std txs before verifying scripts (#10936)
Reject mempool transactions with non-standard transparent inputs before running script verification, avoiding the more expensive script checks and reducing DoS surface. Script verification now runs on the shared Rayon thread pool so it no longer blocks the runtime. Replayed from the private security fix. Closes GHSA-84j3-rw4c-gqmj. Thanks to @ouicate for reporting. Co-authored-by: Conrado Gouvea <conradoplg@gmail.com>
1 parent 11f8a24 commit 2f1fd85

10 files changed

Lines changed: 633 additions & 324 deletions

File tree

CHANGELOG.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,13 @@ and this project adheres to [Semantic Versioning](https://semver.org).
3434
had caught up past the node's non-finalized root would re-subscribe endlessly
3535
instead of syncing, advancing only one block per newly mined block
3636
([#10841](https://github.com/ZcashFoundation/zebra/pull/10841))
37+
- Mempool transactions with non-standard transparent inputs are now rejected
38+
_before_ script verification, to avoid the more expensive script verification
39+
and reduce DoS surface
40+
([GHSA-84j3-rw4c-gqmj](https://github.com/ZcashFoundation/zebra/security/advisories/GHSA-84j3-rw4c-gqmj)).
41+
Thanks to @ouicate for reporting the issue.
42+
- Related to the previous item, script verification now runs on the shared Rayon
43+
thread pool to avoid blocking the runtime.
3744

3845
## [Zebra 6.0.0-rc.0](https://github.com/ZcashFoundation/zebra/releases/tag/v6.0.0-rc.0) - 2026-07-02
3946

zebra-consensus/CHANGELOG.md

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,33 @@ All notable changes to this project will be documented in this file.
55
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
66
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
77

8+
## [Unreleased]
9+
10+
### Breaking Changes
11+
12+
- Added new variants of `error::TransactionError`:
13+
- `NonStandardScriptSigSize`
14+
- `NonStandardScriptSigNotPushOnly`
15+
- `NonStandardInputs`
16+
17+
### Added
18+
19+
- `transaction::check`:
20+
- `mempool_standard_input_scripts`, the pre-verification mempool input-script gate
21+
- `are_inputs_standard` and `standard_script_kind` (zcashd's `AreInputsStandard()` and
22+
scriptPubKey classifier, moved here from `zebrad`)
23+
- the `MAX_P2SH_SIGOPS` and `MAX_STANDARD_SCRIPTSIG_SIZE` policy constants
24+
25+
### Fixed
26+
27+
- Mempool transactions with non-standard transparent inputs are now rejected
28+
_before_ script verification, to avoid the more expensive script verification
29+
and reduce DoS surface
30+
([GHSA-84j3-rw4c-gqmj](https://github.com/ZcashFoundation/zebra/security/advisories/GHSA-84j3-rw4c-gqmj)).
31+
Thanks to @ouicate for reporting the issue.
32+
- Related to the previous item, script verification now runs on the shared Rayon
33+
thread pool to avoid blocking the runtime.
34+
835
## [10.0.0] - 2026-07-02
936

1037
### Added

zebra-consensus/src/error.rs

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ use zebra_chain::{
1919
};
2020
use zebra_state::ValidateContextError;
2121

22-
use crate::{block::MAX_BLOCK_SIGOPS, BoxError};
22+
use crate::{block::MAX_BLOCK_SIGOPS, transaction::check::MAX_STANDARD_SCRIPTSIG_SIZE, BoxError};
2323

2424
#[cfg(any(test, feature = "proptest-impl"))]
2525
use proptest_derive::Arbitrary;
@@ -242,6 +242,20 @@ pub enum TransactionError {
242242
#[cfg_attr(any(test, feature = "proptest-impl"), proptest(skip))]
243243
Zip317(#[from] zebra_chain::transaction::zip317::Error),
244244

245+
// Mempool standardness (policy) rejections, applied before script verification.
246+
// These are not consensus rules: the same input scripts are valid in blocks.
247+
#[error(
248+
"mempool transaction input {input_index} has a {size} byte scriptSig, \
249+
above the {MAX_STANDARD_SCRIPTSIG_SIZE} byte standardness limit"
250+
)]
251+
NonStandardScriptSigSize { input_index: usize, size: usize },
252+
253+
#[error("mempool transaction input {input_index} has a non-push-only scriptSig")]
254+
NonStandardScriptSigNotPushOnly { input_index: usize },
255+
256+
#[error("mempool transaction has non-standard transparent inputs")]
257+
NonStandardInputs,
258+
245259
#[error("transaction uses an incorrect consensus branch id")]
246260
WrongConsensusBranchId,
247261

@@ -390,6 +404,9 @@ impl TransactionError {
390404
| LockedUntilAfterBlockHeight(_)
391405
| LockedUntilAfterBlockTime(_) => 100,
392406

407+
// Standardness (policy) rejections must not be punished: non-standard
408+
// transactions are consensus-valid, and zcashd relays a reject message
409+
// without a DoS score for them.
393410
_other => 0,
394411
}
395412
}

zebra-consensus/src/script.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use tracing::Instrument;
55
use zebra_chain::transparent;
66
use zebra_script::CachedFfiTransaction;
77

8-
use crate::BoxError;
8+
use crate::{primitives::spawn_fifo_and_convert, BoxError};
99

1010
#[cfg(test)]
1111
mod tests;
@@ -58,7 +58,7 @@ impl tower::Service<Request> for Verifier {
5858

5959
let span = tracing::trace_span!("script");
6060
async move {
61-
let input = &cached_ffi_transaction
61+
let input = cached_ffi_transaction
6262
.inputs()
6363
.get(input_index)
6464
.ok_or_else(|| {
@@ -69,8 +69,9 @@ impl tower::Service<Request> for Verifier {
6969
transparent::Input::PrevOut { outpoint, .. } => {
7070
let outpoint = *outpoint;
7171

72-
// Avoid calling the state service if the utxo is already known
73-
cached_ffi_transaction.is_valid(input_index)?;
72+
// Script verification is CPU-bound so run in Rayon thread
73+
spawn_fifo_and_convert(move || cached_ffi_transaction.is_valid(input_index))
74+
.await?;
7475
tracing::trace!(?outpoint, "script verification succeeded");
7576

7677
Ok(())

zebra-consensus/src/transaction.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -456,6 +456,12 @@ where
456456
// the state once #2336 has been implemented?
457457
if req.is_mempool() {
458458
Self::check_maturity_height(&network, &req, &spent_utxos)?;
459+
460+
// Reject non-standard input scripts (oversized or non-push-only
461+
// scriptSigs, and high-sigop P2SH redeem scripts) *before*
462+
// doing expensive script verification, to avoid DoS attacks on
463+
// the script interpreter.
464+
check::mempool_standard_input_scripts(tx.as_ref(), &spent_outputs)?;
459465
}
460466

461467
let nu = req.upgrade(&network);

zebra-consensus/src/transaction/check.rs

Lines changed: 219 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,11 @@ use std::{
1111

1212
use chrono::{DateTime, Utc};
1313

14+
use zcash_script::{
15+
opcode::PossiblyBad,
16+
script::{self, Evaluable as _},
17+
solver, Opcode,
18+
};
1419
use zebra_chain::{
1520
amount::{Amount, NegativeAllowed, NonNegative},
1621
block::Height,
@@ -630,6 +635,220 @@ pub fn tx_transparent_coinbase_spends_maturity(
630635
Ok(())
631636
}
632637

638+
/// The maximum number of signature operations in the redeem script of a standard P2SH input.
639+
///
640+
/// This is zcashd's `MAX_P2SH_SIGOPS` standardness (policy) constant:
641+
/// <https://github.com/zcash/zcash/blob/v6.11.0/src/policy/policy.h#L20>
642+
pub const MAX_P2SH_SIGOPS: u32 = 15;
643+
644+
/// The maximum size in bytes of the scriptSig of a standard transaction input.
645+
///
646+
/// This is zcashd's `MAX_STANDARD_SCRIPTSIG_SIZE` standardness (policy) constant:
647+
/// <https://github.com/zcash/zcash/blob/v6.11.0/src/policy/policy.cpp#L92-L99>
648+
pub const MAX_STANDARD_SCRIPTSIG_SIZE: usize = 1650;
649+
650+
/// Classify a script using the `zcash_script` solver.
651+
///
652+
/// Returns `Some(kind)` for standard script types, `None` for non-standard.
653+
///
654+
/// Mirrors the classification done by zcashd's `Solver()`.
655+
pub fn standard_script_kind(lock_script: &transparent::Script) -> Option<solver::ScriptKind> {
656+
let code = script::Code(lock_script.as_raw_bytes().to_vec());
657+
let component = code.to_component().ok()?.refine().ok()?;
658+
solver::standard(&component)
659+
}
660+
661+
/// Returns the expected number of scriptSig arguments for a given script kind.
662+
///
663+
/// Mirrors zcashd's `ScriptSigArgsExpected()`:
664+
/// <https://github.com/zcash/zcash/blob/v6.11.0/src/script/standard.cpp#L135>
665+
///
666+
/// Returns `None` for non-standard types (TX_NONSTANDARD, TX_NULL_DATA).
667+
pub(super) fn script_sig_args_expected(kind: &solver::ScriptKind) -> Option<usize> {
668+
match kind {
669+
solver::ScriptKind::PubKey { .. } => Some(1),
670+
solver::ScriptKind::PubKeyHash { .. } => Some(2),
671+
solver::ScriptKind::ScriptHash { .. } => Some(1),
672+
solver::ScriptKind::MultiSig { required, .. } => Some(*required as usize + 1),
673+
solver::ScriptKind::NullData { .. } => None,
674+
}
675+
}
676+
677+
/// Extract the redeemed script bytes from a P2SH scriptSig.
678+
///
679+
/// The redeemed script is the last data push in the scriptSig.
680+
/// Returns `None` if the scriptSig has no push operations.
681+
///
682+
/// # Precondition
683+
///
684+
/// The scriptSig should be push-only (enforced by [`mempool_standard_input_scripts`] before this
685+
/// function is reached). Non-push opcodes are silently ignored.
686+
pub(super) fn extract_p2sh_redeemed_script(unlock_script: &transparent::Script) -> Option<Vec<u8>> {
687+
let code = script::Code(unlock_script.as_raw_bytes().to_vec());
688+
let mut last_push_data: Option<Vec<u8>> = None;
689+
for opcode in code.parse().flatten() {
690+
if let PossiblyBad::Good(Opcode::PushValue(pv)) = opcode {
691+
last_push_data = Some(pv.value());
692+
}
693+
}
694+
last_push_data
695+
}
696+
697+
/// Count the number of push operations in a script.
698+
///
699+
/// For a push-only script (already enforced for mempool scriptSigs),
700+
/// this equals the stack depth after evaluation.
701+
pub(super) fn count_script_push_ops(script_bytes: &[u8]) -> usize {
702+
let code = script::Code(script_bytes.to_vec());
703+
code.parse()
704+
.filter(|op| matches!(op, Ok(PossiblyBad::Good(Opcode::PushValue(_)))))
705+
.count()
706+
}
707+
708+
/// Returns `true` if all of a transaction's transparent inputs are standard.
709+
///
710+
/// Mirrors zcashd's `AreInputsStandard()`:
711+
/// <https://github.com/zcash/zcash/blob/v6.11.0/src/policy/policy.cpp#L136>
712+
///
713+
/// For each input:
714+
/// 1. The spent output's scriptPubKey must be a known standard type (via the `zcash_script`
715+
/// solver). Non-standard scripts and OP_RETURN outputs are rejected.
716+
/// 2. The scriptSig stack depth must match `ScriptSigArgsExpected()`.
717+
/// 3. For P2SH inputs:
718+
/// - If the redeemed script is standard, its expected args are added to the total.
719+
/// - If the redeemed script is non-standard, it must have at most [`MAX_P2SH_SIGOPS`] sigops.
720+
///
721+
/// # Correctness
722+
///
723+
/// Callers must ensure `spent_outputs.len()` matches the number of transparent inputs.
724+
/// If the lengths differ, `false` is returned.
725+
pub fn are_inputs_standard(tx: &Transaction, spent_outputs: &[transparent::Output]) -> bool {
726+
if tx.inputs().len() != spent_outputs.len() {
727+
return false;
728+
}
729+
for (input, spent_output) in tx.inputs().iter().zip(spent_outputs.iter()) {
730+
let unlock_script = match input {
731+
transparent::Input::PrevOut { unlock_script, .. } => unlock_script,
732+
transparent::Input::Coinbase { .. } => continue,
733+
};
734+
735+
// Step 1: Classify the spent output's scriptPubKey via the zcash_script solver.
736+
let script_kind = match standard_script_kind(&spent_output.lock_script) {
737+
Some(kind) => kind,
738+
None => return false,
739+
};
740+
741+
// Step 2: Get expected number of scriptSig arguments.
742+
// Returns None for TX_NONSTANDARD and TX_NULL_DATA.
743+
let mut n_args_expected = match script_sig_args_expected(&script_kind) {
744+
Some(n) => n,
745+
None => return false,
746+
};
747+
748+
// Step 3: Count actual push operations in scriptSig.
749+
// For push-only scripts (enforced before this function), this equals the stack depth.
750+
let stack_size = count_script_push_ops(unlock_script.as_raw_bytes());
751+
752+
// Step 4: P2SH-specific checks.
753+
if matches!(script_kind, solver::ScriptKind::ScriptHash { .. }) {
754+
let Some(redeemed_bytes) = extract_p2sh_redeemed_script(unlock_script) else {
755+
return false;
756+
};
757+
758+
let redeemed_code = script::Code(redeemed_bytes);
759+
760+
// Classify the redeemed script using the zcash_script solver.
761+
let redeemed_kind = {
762+
let component = redeemed_code
763+
.to_component()
764+
.ok()
765+
.and_then(|c| c.refine().ok());
766+
component.and_then(|c| solver::standard(&c))
767+
};
768+
769+
match redeemed_kind {
770+
Some(ref inner_kind) => {
771+
// Standard redeemed script: add its expected args.
772+
match script_sig_args_expected(inner_kind) {
773+
Some(inner) => n_args_expected += inner,
774+
None => return false,
775+
}
776+
}
777+
None => {
778+
// Non-standard redeemed script: accept if sigops <= limit.
779+
// Matches zcashd: "Any other Script with less than 15 sigops OK:
780+
// ... extra data left on the stack after execution is OK, too"
781+
let sigops = redeemed_code.sig_op_count(true);
782+
if sigops > MAX_P2SH_SIGOPS {
783+
return false;
784+
}
785+
786+
// This input is acceptable; move on to the next input.
787+
continue;
788+
}
789+
}
790+
}
791+
792+
// Step 5: Reject if scriptSig has wrong number of stack items.
793+
if stack_size != n_args_expected {
794+
return false;
795+
}
796+
}
797+
true
798+
}
799+
800+
/// Standardness (policy) checks on a mempool transaction's transparent input scripts, applied
801+
/// *before* the transaction is dispatched to script verification. The goal is to avoid the
802+
/// expensive verification for non-standard transactions which would be rejected anyway
803+
/// by `Storage::reject_if_non_standard_tx()`; this is a subset of the checks
804+
/// in that function.
805+
///
806+
/// `spent_outputs` must contain the output spent by each of the transaction's transparent inputs,
807+
/// in input order.
808+
///
809+
/// # Correctness
810+
///
811+
/// `spent_outputs.len()` must equal the number of transparent inputs in `tx`: if the lengths
812+
/// differ, `zip()` silently truncates, and some inputs are not checked.
813+
pub fn mempool_standard_input_scripts(
814+
tx: &Transaction,
815+
spent_outputs: &[transparent::Output],
816+
) -> Result<(), TransactionError> {
817+
if tx.inputs().len() != spent_outputs.len() {
818+
return Err(TransactionError::Other(format!(
819+
"spent_outputs must align with transaction inputs for non-coinbase txs: inputs={}, spent_outputs={}",
820+
tx.inputs().len(),
821+
spent_outputs.len(),
822+
)));
823+
}
824+
825+
for (input_index, input) in tx.inputs().iter().enumerate() {
826+
let unlock_script = match input {
827+
transparent::Input::PrevOut { unlock_script, .. } => unlock_script,
828+
transparent::Input::Coinbase { .. } => continue,
829+
};
830+
831+
// Rule: the scriptSig must be within the standard size limit.
832+
let size = unlock_script.as_raw_bytes().len();
833+
if size > MAX_STANDARD_SCRIPTSIG_SIZE {
834+
return Err(TransactionError::NonStandardScriptSigSize { input_index, size });
835+
}
836+
837+
// Rule: the scriptSig must be push-only.
838+
if !script::Code(unlock_script.as_raw_bytes().to_vec()).is_push_only() {
839+
return Err(TransactionError::NonStandardScriptSigNotPushOnly { input_index });
840+
}
841+
}
842+
843+
// Rule: all transparent inputs must pass `AreInputsStandard()` checks:
844+
// https://github.com/zcash/zcash/blob/v6.11.0/src/policy/policy.cpp#L137
845+
if !are_inputs_standard(tx, spent_outputs) {
846+
return Err(TransactionError::NonStandardInputs);
847+
}
848+
849+
Ok(())
850+
}
851+
633852
/// Checks the `nConsensusBranchId` field.
634853
///
635854
/// # Consensus

0 commit comments

Comments
 (0)