Merge rust-bitcoin/rust-bitcoin#6805: crypto: Add NonStandardSighashType wrapper
What changed, and why it matters
This change is a code-quality and type-safety improvement, not a security fix. It replaces a raw number in the 'NonStandard' sighash variant with a dedicated wrapper type so that the variant can only hold genuinely non-standard values. This prevents accidental misuse where a standard sighash type could be represented both as a normal enum variant and as a 'NonStandard' number, but it does not close an exploitable vulnerability on its own.
No urgent action required. Treat as a normal refactor/API-improvement. Review downstream code that pattern-matches on EcdsaSighashType::NonStandard(_) or constructs it directly, because direct construction is no longer possible; use EcdsaSighashType::from_consensus instead.
Security signals we found
Type-system hardening to prevent representation of standard sighash types as NonStandard
No memory-safety, cryptographic, or consensus bug is fixed in the diff
No advisory, CVE, or security disclosure is referenced in the commit or PR
Evidence from the diff
The commit refactors EcdsaSighashType::NonStandard(u32) into EcdsaSighashType::NonStandard(NonStandardSighashType), where NonStandardSighashType is a new tuple struct with a private inner u32. from_consensus now maps the six standard byte values to the standard enum variants and only wraps other values in NonStandardSighashType. The enum derives PartialEq, Eq, and Hash instead of implementing them via to_u32, and call sites are updated to use to_u32() to access the inner value. The change is purely defensive API hardening.
Changed components
crypto/src/sighash.rsbitcoin/src/crypto/sighash.rsInspect captured patch +28 / −28
### bitcoin/src/crypto/sighash.rs
@@ -39,7 +39,7 @@ pub use self::error::{
PrevoutsSizeError, TaprootError,
};
#[doc(inline)]
-pub use crypto::sighash::{EcdsaSighashType, TapSighashType};
+pub use crypto::sighash::{EcdsaSighashType, NonStandardSighashType, TapSighashType};
/// Used for signature hash for invalid use of SIGHASH_SINGLE.
#[rustfmt::skip]
@@ -309,6 +309,7 @@ impl SplitAnyoneCanPay for EcdsaSighashType {
NonePlusAnyoneCanPay => (None, true),
SinglePlusAnyoneCanPay => (Single, true),
NonStandard(n) => {
+ let n = n.to_u32();
// Check sighash tyoe
let sighash_type = match n & 0x1f {
0x02 => None,
@@ -1947,7 +1948,7 @@ mod tests {
let witness = &tx.inputs[0].witness;
let sig = crate::ecdsa::Signature::from_slice(witness.get(0).unwrap())
.expect("non-standard sighash types parse");
- assert_eq!(sig.sighash_type, EcdsaSighashType::NonStandard(0x65));
+ assert_eq!(sig.sighash_type, EcdsaSighashType::from_consensus(0x65));
let pk = PublicKey::from_slice(witness.get(1).unwrap()).unwrap();
// redeemScript from the scriptSig: the v0 witness program.
### crypto/src/sighash.rs
@@ -99,7 +99,7 @@ impl TapSighashType {
}
/// Hashtype of an input's signature, encoded in the last byte of the signature.
-#[derive(Copy, Clone, Debug)]
+#[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)]
pub enum EcdsaSighashType {
/// 0x1: Sign all outputs.
All,
@@ -122,17 +122,23 @@ pub enum EcdsaSighashType {
/// sighash type as 4 bytes, even though only the lowest byte is appended to the
/// signature in a transaction. On bitcoin the higher bits are always zero, but on replay-protected forks they are
/// sometimes set.
- NonStandard(u32),
+ NonStandard(NonStandardSighashType),
}
-#[cfg(feature = "serde")]
-internals::serde_string_impl!(EcdsaSighashType, "a EcdsaSighashType data");
-impl PartialEq for EcdsaSighashType {
+/// A consensus-valid sighash type that is not one of the six standard values.
+///
+/// The inner `u32` is private so that [`EcdsaSighashType::NonStandard`] cannot represent a
+/// standard sighash type. Users who need a non-standard sighash type can use [`EcdsaSighashType::from_consensus`].
+#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)]
+pub struct NonStandardSighashType(u32);
+
+impl NonStandardSighashType {
+ /// Converts [`NonStandardSighashType`] to a `u32` sighash flag.
#[inline]
- fn eq(&self, other: &Self) -> bool { self.to_u32() == other.to_u32() }
+ pub fn to_u32(self) -> u32 { self.0 }
}
-
-impl Eq for EcdsaSighashType {}
+#[cfg(feature = "serde")]
+internals::serde_string_impl!(EcdsaSighashType, "a EcdsaSighashType data");
impl PartialOrd for EcdsaSighashType {
#[inline]
@@ -144,13 +150,6 @@ impl Ord for EcdsaSighashType {
fn cmp(&self, other: &Self) -> core::cmp::Ordering { self.to_u32().cmp(&other.to_u32()) }
}
-impl core::hash::Hash for EcdsaSighashType {
- #[inline]
- fn hash<H: core::hash::Hasher>(&self, state: &mut H) {
- core::hash::Hash::hash(&self.to_u32(), state);
- }
-}
-
impl fmt::Display for EcdsaSighashType {
#[inline]
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
@@ -161,7 +160,7 @@ impl fmt::Display for EcdsaSighashType {
Self::AllPlusAnyoneCanPay => "SIGHASH_ALL|SIGHASH_ANYONECANPAY",
Self::NonePlusAnyoneCanPay => "SIGHASH_NONE|SIGHASH_ANYONECANPAY",
Self::SinglePlusAnyoneCanPay => "SIGHASH_SINGLE|SIGHASH_ANYONECANPAY",
- Self::NonStandard(n) => return write!(f, "0x{:02x}", n),
+ Self::NonStandard(n) => return write!(f, "0x{:02x}", n.to_u32()),
};
f.write_str(s)
}
@@ -206,7 +205,7 @@ impl EcdsaSighashType {
match *self {
Self::Single => true,
Self::SinglePlusAnyoneCanPay => true,
- Self::NonStandard(n) => (n & 0x1f) == 0x03,
+ Self::NonStandard(n) => (n.to_u32() & 0x1f) == 0x03,
_ => false,
}
}
@@ -227,7 +226,7 @@ impl EcdsaSighashType {
0x81 => Self::AllPlusAnyoneCanPay,
0x82 => Self::NonePlusAnyoneCanPay,
0x83 => Self::SinglePlusAnyoneCanPay,
- other => Self::NonStandard(other),
+ other => Self::NonStandard(NonStandardSighashType(other)),
}
}
@@ -260,7 +259,7 @@ impl EcdsaSighashType {
Self::AllPlusAnyoneCanPay => 0x81,
Self::NonePlusAnyoneCanPay => 0x82,
Self::SinglePlusAnyoneCanPay => 0x83,
- Self::NonStandard(n) => n,
+ Self::NonStandard(n) => n.to_u32(),
}
}
@@ -285,7 +284,7 @@ impl TryFrom<EcdsaSighashType> for TapSighashType {
EcdsaSighashType::NonePlusAnyoneCanPay => Ok(Self::NonePlusAnyoneCanPay),
EcdsaSighashType::SinglePlusAnyoneCanPay => Ok(Self::SinglePlusAnyoneCanPay),
// Taproot doesnt accept non-standard sighash
- EcdsaSighashType::NonStandard(n) => Err(InvalidSighashTypeError(n)),
+ EcdsaSighashType::NonStandard(n) => Err(InvalidSighashTypeError(n.to_u32())),
}
}
}
@@ -470,13 +469,13 @@ mod tests {
match n {
0x01 | 0x02 | 0x03 | 0x81 | 0x82 | 0x83 =>
assert!(!matches!(ty, EcdsaSighashType::NonStandard(_))),
- _ => assert_eq!(ty, EcdsaSighashType::NonStandard(n)),
+ _ => assert!(matches!(ty, EcdsaSighashType::NonStandard(_))),
}
}
// On replay-protected forks bits above the lowest byte are sometimes set.
let ty = EcdsaSighashType::from_consensus(0x0100_0041);
- assert_eq!(ty, EcdsaSighashType::NonStandard(0x0100_0041));
+ assert!(matches!(ty, EcdsaSighashType::NonStandard(_)));
assert_eq!(ty.to_u32(), 0x0100_0041);
assert_eq!(ty.to_consensus_u8(), 0x41);
@@ -486,10 +485,10 @@ mod tests {
fn ecdsa_sighash_type_non_standard_is_single_uses_mask() {
use super::EcdsaSighashType;
- assert!(EcdsaSighashType::NonStandard(0x63).is_single());
- assert!(EcdsaSighashType::NonStandard(0xe3).is_single());
- assert!(!EcdsaSighashType::NonStandard(0x65).is_single());
- assert!(!EcdsaSighashType::NonStandard(0x62).is_single());
+ assert!(EcdsaSighashType::from_consensus(0x63).is_single());
+ assert!(EcdsaSighashType::from_consensus(0xe3).is_single());
+ assert!(!EcdsaSighashType::from_consensus(0x65).is_single());
+ assert!(!EcdsaSighashType::from_consensus(0x62).is_single());
}
#[test]Why this scored 19/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.