Skip to content

sign_input should respect allow_all_sighashes sign option #469

@ValuedMammal

Description

@ValuedMammal

Description

SignOptions::allow_all_sighashes is documented as controlling whether the signer will accept non-SIGHASH_ALL sighash types. However, this flag is only checked inside Wallet::sign as a PSBT-wide pre-flight guard. The underlying InputSigner::sign_input never consults it although it's a public trait method. Callers that use InputSigner or TransactionSigner directly bypass the check entirely.

Affected code

  • SignerWrapper<PrivateKey>::sign_input — the leaf implementation for all BDK software signers.
    DescriptorXKey and DescriptorMultiXKey both derive a key and then delegate here, so all three are affected through the same code path.
  • All signing contexts (Legacy, Segwitv0, Taproot) are affected.

The ECDSA path calls psbt.sighash_ecdsa() which returns whatever type the PSBT declares and proceeds. The Taproot path calls compute_tap_sighash() which similarly reads the PSBT sighash type without checking allow_all_sighashes.

Steps to reproduce

  1. Obtain a signer directly (e.g. from wallet internals or a custom SignersContainer)
  2. Set SIGHASH_NONE on a PSBT input
  3. Call signer.sign_input(&mut psbt, 0, &SignOptions::default(), &secp) (signs successfully despite allow_all_sighashes being false)

Expected behavior

sign_input should return Err(SignerError::NonStandardSighash) when sign_options.allow_all_sighashes is false and the PSBT input declares a sighash type other than SIGHASH_ALL / SIGHASH_DEFAULT.

Proposed fix

Add the sighash guard in SignerWrapper<PrivateKey>::sign_input, after the "already finalized" early return but before the match self.ctx dispatch. This ensures the check covers all signing contexts and all delegating implementations in a single place:

if !sign_options.allow_all_sighashes {
    let sighash_type = psbt.inputs[input_index].sighash_type;
    if sighash_type.is_some()
        && sighash_type != Some(EcdsaSighashType::All.into())
        && sighash_type != Some(TapSighashType::All.into())
        && sighash_type != Some(TapSighashType::Default.into())
    {
        return Err(SignerError::NonStandardSighash);
    }
}

This mirrors the existing logic in Wallet::sign. Through the Wallet::sign path, behavior is unchanged since the PSBT-wide check still fires first.

A test should be added covering:

  • Taproot input with SIGHASH_NONE and allow_all_sighashes: false returns NonStandardSighash
  • The same with allow_all_sighashes: true proceeds normally.

Metadata

Metadata

Assignees

No one assigned

    Labels

    auditSuggested as result of external code auditbugSomething isn't working

    Type

    No type

    Projects

    Status

    No status

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions