Security: Stop panicking on invalid reserved orchard::Flags bits (#2284)
* stop panicking in deserialize orchard flags * make the test simpler Co-authored-by: Janito Vaqueiro Ferreira Filho <janito.vff@gmail.com>
This commit is contained in:
parent
86f23f7960
commit
3dcd407d66
|
@ -416,6 +416,9 @@ bitflags! {
|
||||||
|
|
||||||
This type is also defined in `orchard/shielded_data.rs`.
|
This type is also defined in `orchard/shielded_data.rs`.
|
||||||
|
|
||||||
|
Note: A [consensus rule](https://zips.z.cash/protocol/protocol.pdf#txnencodingandconsensus) was added to the protocol specification stating that:
|
||||||
|
> In a version 5 transaction, the reserved bits 2..7 of the flagsOrchard field MUST be zero.
|
||||||
|
|
||||||
## Test Plan
|
## Test Plan
|
||||||
[test-plan]: #test-plan
|
[test-plan]: #test-plan
|
||||||
|
|
||||||
|
|
|
@ -133,6 +133,14 @@ bitflags! {
|
||||||
///
|
///
|
||||||
/// The spend and output flags are passed to the `Halo2Proof` verifier, which verifies
|
/// The spend and output flags are passed to the `Halo2Proof` verifier, which verifies
|
||||||
/// the relevant note spending and creation consensus rules.
|
/// the relevant note spending and creation consensus rules.
|
||||||
|
///
|
||||||
|
/// Consensus rules:
|
||||||
|
///
|
||||||
|
/// - "In a version 5 transaction, the reserved bits 2..7 of the flagsOrchard field MUST be zero."
|
||||||
|
/// ([`bitflags`](https://docs.rs/bitflags/1.2.1/bitflags/index.html) restricts its values to the
|
||||||
|
/// set of valid flags)
|
||||||
|
/// - "In a version 5 coinbase transaction, the enableSpendsOrchard flag MUST be 0."
|
||||||
|
/// (Checked in zebra-consensus)
|
||||||
#[derive(Deserialize, Serialize)]
|
#[derive(Deserialize, Serialize)]
|
||||||
pub struct Flags: u8 {
|
pub struct Flags: u8 {
|
||||||
/// Enable spending non-zero valued Orchard notes.
|
/// Enable spending non-zero valued Orchard notes.
|
||||||
|
@ -141,7 +149,6 @@ bitflags! {
|
||||||
const ENABLE_SPENDS = 0b00000001;
|
const ENABLE_SPENDS = 0b00000001;
|
||||||
/// Enable creating new non-zero valued Orchard notes.
|
/// Enable creating new non-zero valued Orchard notes.
|
||||||
const ENABLE_OUTPUTS = 0b00000010;
|
const ENABLE_OUTPUTS = 0b00000010;
|
||||||
// Reserved, zeros (bits 2 .. 7)
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -155,8 +162,10 @@ impl ZcashSerialize for Flags {
|
||||||
|
|
||||||
impl ZcashDeserialize for Flags {
|
impl ZcashDeserialize for Flags {
|
||||||
fn zcash_deserialize<R: io::Read>(mut reader: R) -> Result<Self, SerializationError> {
|
fn zcash_deserialize<R: io::Read>(mut reader: R) -> Result<Self, SerializationError> {
|
||||||
let flags = Flags::from_bits(reader.read_u8()?).unwrap();
|
// Consensus rule: "In a version 5 transaction,
|
||||||
|
// the reserved bits 2..7 of the flagsOrchard field MUST be zero."
|
||||||
Ok(flags)
|
// https://zips.z.cash/protocol/protocol.pdf#txnencodingandconsensus
|
||||||
|
Flags::from_bits(reader.read_u8()?)
|
||||||
|
.ok_or(SerializationError::Parse("invalid reserved orchard flags"))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -1 +1,2 @@
|
||||||
mod preallocate;
|
mod preallocate;
|
||||||
|
mod prop;
|
||||||
|
|
|
@ -0,0 +1,35 @@
|
||||||
|
use proptest::prelude::*;
|
||||||
|
use std::io::Cursor;
|
||||||
|
|
||||||
|
use crate::{
|
||||||
|
orchard,
|
||||||
|
serialization::{ZcashDeserializeInto, ZcashSerialize},
|
||||||
|
};
|
||||||
|
|
||||||
|
proptest! {
|
||||||
|
/// Make sure only valid flags deserialize
|
||||||
|
#[test]
|
||||||
|
fn flag_roundtrip_bytes(flags in any::<u8>()) {
|
||||||
|
|
||||||
|
let mut serialized = Cursor::new(Vec::new());
|
||||||
|
flags.zcash_serialize(&mut serialized)?;
|
||||||
|
|
||||||
|
serialized.set_position(0);
|
||||||
|
let maybe_deserialized = (&mut serialized).zcash_deserialize_into();
|
||||||
|
|
||||||
|
let invalid_bits_mask = !orchard::Flags::all().bits();
|
||||||
|
match orchard::Flags::from_bits(flags) {
|
||||||
|
Some(valid_flags) => {
|
||||||
|
prop_assert_eq!(maybe_deserialized.ok(), Some(valid_flags));
|
||||||
|
prop_assert_eq!(flags & invalid_bits_mask, 0);
|
||||||
|
}
|
||||||
|
None => {
|
||||||
|
prop_assert_eq!(
|
||||||
|
maybe_deserialized.err().unwrap().to_string(),
|
||||||
|
"parse error: invalid reserved orchard flags"
|
||||||
|
);
|
||||||
|
prop_assert_ne!(flags & invalid_bits_mask, 0);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
Loading…
Reference in New Issue