From 2e6d3d9fdb5c80980d06a8d628d7de236c86fdc0 Mon Sep 17 00:00:00 2001 From: GarrickOllivander Date: Sat, 17 Dec 2016 21:44:14 +0000 Subject: [PATCH] Safer serialization of base types (#18) * use MESSAGE_SIZE constant instead of 32 * use Hashed trait in genesis * safer serialization of Hash * Hashed of [u8] should send only data to hash function, not length + data * Safer serialization of Proof * Safer serialization of Commitment * Safer serialization of RangeProof * introduce read_limited_vec instead of potential panic in conversion --- chain/src/types.rs | 8 +++--- core/src/core/block.rs | 35 +++++++++++-------------- core/src/core/hash.rs | 24 +++++++++-------- core/src/core/mod.rs | 45 +++++++++++++++----------------- core/src/core/transaction.rs | 16 +++++++----- core/src/genesis.rs | 12 +++------ core/src/ser.rs | 50 ++++++++++++++++++++++++++++-------- secp256k1zkp/src/pedersen.rs | 21 +-------------- 8 files changed, 106 insertions(+), 105 deletions(-) diff --git a/chain/src/types.rs b/chain/src/types.rs index dc499d788..22ad9b633 100644 --- a/chain/src/types.rs +++ b/chain/src/types.rs @@ -111,13 +111,13 @@ impl ser::Writeable for Tip { impl ser::Readable for Tip { fn read(reader: &mut ser::Reader) -> Result { let height = try!(reader.read_u64()); - let last = try!(reader.read_fixed_bytes(32)); - let prev = try!(reader.read_fixed_bytes(32)); + let last = try!(Hash::read(reader)); + let prev = try!(Hash::read(reader)); let line = try!(Lineage::read(reader)); Ok(Tip { height: height, - last_block_h: Hash::from_vec(last), - prev_block_h: Hash::from_vec(prev), + last_block_h: last, + prev_block_h: prev, lineage: line, }) } diff --git a/core/src/core/block.rs b/core/src/core/block.rs index 7c02a6c28..610235951 100644 --- a/core/src/core/block.rs +++ b/core/src/core/block.rs @@ -23,7 +23,7 @@ use std::collections::HashSet; use core::Committed; use core::{Input, Output, Proof, TxProof, Transaction}; use core::transaction::merkle_inputs_outputs; -use consensus::{PROOFSIZE, REWARD, DEFAULT_SIZESHIFT, MAX_TARGET}; +use consensus::{REWARD, DEFAULT_SIZESHIFT, MAX_TARGET}; use core::hash::{Hash, Hashed, ZERO_HASH}; use core::target::Target; use ser::{self, Readable, Reader, Writeable, Writer}; @@ -79,40 +79,35 @@ impl Writeable for BlockHeader { // make sure to not introduce any variable length data before the nonce to // avoid complicating PoW try!(writer.write_u64(self.nonce)); - // cuckoo cycle of 42 nodes - for n in 0..PROOFSIZE { - try!(writer.write_u32(self.pow.0[n])); - } - Ok(()) + // proof + self.pow.write(writer) } } /// Deserialization of a block header impl Readable for BlockHeader { fn read(reader: &mut Reader) -> Result { - let (height, previous, timestamp, cuckoo_len) = - ser_multiread!(reader, read_u64, read_32_bytes, read_i64, read_u8); + let height = try!(reader.read_u64()); + let previous = try!(Hash::read(reader)); + let (timestamp, cuckoo_len) = ser_multiread!(reader, read_i64, read_u8); let target = try!(Target::read(reader)); - let (utxo_merkle, tx_merkle, nonce) = - ser_multiread!(reader, read_32_bytes, read_32_bytes, read_u64); + let utxo_merkle = try!(Hash::read(reader)); + let tx_merkle = try!(Hash::read(reader)); + let nonce = try!(reader.read_u64()); + let pow = try!(Proof::read(reader)); - // cuckoo cycle of 42 nodes - let mut pow = [0; PROOFSIZE]; - for n in 0..PROOFSIZE { - pow[n] = try!(reader.read_u32()); - } Ok(BlockHeader { height: height, - previous: Hash::from_vec(previous), + previous: previous, timestamp: time::at_utc(time::Timespec { sec: timestamp, nsec: 0, }), cuckoo_len: cuckoo_len, target: target, - utxo_merkle: Hash::from_vec(utxo_merkle), - tx_merkle: Hash::from_vec(tx_merkle), - pow: Proof(pow), + utxo_merkle: utxo_merkle, + tx_merkle: tx_merkle, + pow: pow, nonce: nonce, }) } @@ -356,7 +351,7 @@ impl Block { fn reward_output(skey: secp::key::SecretKey, secp: &Secp256k1) -> Result<(Output, TxProof), secp::Error> { - let msg = try!(secp::Message::from_slice(&[0; 32])); + let msg = try!(secp::Message::from_slice(&[0; secp::constants::MESSAGE_SIZE])); let sig = try!(secp.sign(&msg, &skey)); let output = Output::OvertOutput { value: REWARD, diff --git a/core/src/core/hash.rs b/core/src/core/hash.rs index 55ba3358d..7987e7803 100644 --- a/core/src/core/hash.rs +++ b/core/src/core/hash.rs @@ -21,7 +21,7 @@ use byteorder::{ByteOrder, BigEndian}; use std::fmt; use tiny_keccak::Keccak; -use ser::{self, AsFixedBytes}; +use ser::{self, AsFixedBytes, Reader, Readable, Error}; pub const ZERO_HASH: Hash = Hash([0; 32]); @@ -40,14 +40,6 @@ impl fmt::Display for Hash { } impl Hash { - /// Creates a new hash from a vector - pub fn from_vec(v: Vec) -> Hash { - let mut a = [0; 32]; - for i in 0..a.len() { - a[i] = v[i]; - } - Hash(a) - } /// Converts the hash to a byte vector pub fn to_vec(&self) -> Vec { self.0.to_vec() @@ -58,6 +50,18 @@ impl Hash { } } + +impl Readable for Hash { + fn read(reader: &mut Reader) -> Result { + let v = try!(reader.read_fixed_bytes(32)); + let mut a = [0; 32]; + for i in 0..a.len() { + a[i] = v[i]; + } + Ok(Hash(a)) + } +} + /// Serializer that outputs a hash of the serialized object pub struct HashWriter { state: Keccak, @@ -105,7 +109,7 @@ impl Hashed for [u8] { fn hash(&self) -> Hash { let mut hasher = HashWriter::default(); let mut ret = [0; 32]; - ser::Writer::write_bytes(&mut hasher, &self).unwrap(); + ser::Writer::write_fixed_bytes(&mut hasher, &self).unwrap(); hasher.finalize(&mut ret); Hash(ret) } diff --git a/core/src/core/mod.rs b/core/src/core/mod.rs index ddc22115c..e3244f714 100644 --- a/core/src/core/mod.rs +++ b/core/src/core/mod.rs @@ -30,7 +30,7 @@ use consensus::PROOFSIZE; pub use self::block::{Block, BlockHeader}; pub use self::transaction::{Transaction, Input, Output, TxProof}; use self::hash::{Hash, Hashed, HashWriter, ZERO_HASH}; -use ser::{Writeable, Writer, Error}; +use ser::{Writeable, Writer, Reader, Readable, Error}; /// Implemented by types that hold inputs and outputs including Pedersen /// commitments. Handles the collection of the commitments as well as their @@ -109,32 +109,10 @@ impl Clone for Proof { } } -impl Hashed for Proof { - fn hash(&self) -> Hash { - let mut hasher = HashWriter::default(); - let mut ret = [0; 32]; - for p in self.0.iter() { - hasher.write_u32(*p).unwrap(); - } - hasher.finalize(&mut ret); - Hash(ret) - } -} - impl Proof { /// Builds a proof with all bytes zeroed out pub fn zero() -> Proof { - Proof([0; 42]) - } - - /// Builds a proof from a vector of exactly PROOFSIZE (42) u32. - pub fn from_vec(v: Vec) -> Proof { - assert!(v.len() == PROOFSIZE); - let mut p = [0; PROOFSIZE]; - for (n, elem) in v.iter().enumerate() { - p[n] = *elem; - } - Proof(p) + Proof([0; PROOFSIZE]) } /// Converts the proof to a vector of u64s @@ -158,6 +136,25 @@ impl Proof { } } +impl Readable for Proof { + fn read(reader: &mut Reader) -> Result { + let mut pow = [0u32; PROOFSIZE]; + for n in 0..PROOFSIZE { + pow[n] = try!(reader.read_u32()); + } + Ok(Proof(pow)) + } +} + +impl Writeable for Proof { + fn write(&self, writer: &mut Writer) -> Result<(), Error> { + for n in 0..PROOFSIZE { + try!(writer.write_u32(self.0[n])); + } + Ok(()) + } +} + /// Two hashes that will get hashed together in a Merkle tree to build the next /// level up. struct HPair(Hash, Hash); diff --git a/core/src/core/transaction.rs b/core/src/core/transaction.rs index d05ebfb06..c938107b7 100644 --- a/core/src/core/transaction.rs +++ b/core/src/core/transaction.rs @@ -51,9 +51,10 @@ impl Writeable for TxProof { impl Readable for TxProof { fn read(reader: &mut Reader) -> Result { - let (remainder, sig, fee) = ser_multiread!(reader, read_33_bytes, read_vec, read_u64); + let remainder = try!(Commitment::read (reader)); + let (sig, fee) = ser_multiread!(reader, read_vec, read_u64); Ok(TxProof { - remainder: Commitment::from_vec(remainder), + remainder: remainder, sig: sig, fee: fee, }) @@ -257,8 +258,8 @@ impl Writeable for Input { /// an Input from a binary stream. impl Readable for Input { fn read(reader: &mut Reader) -> Result { - reader.read_fixed_bytes(32) - .map(|h| Input::BareInput { output: Hash::from_vec(h) }) + Hash::read(reader) + .map(|h| Input::BareInput { output: h }) } } @@ -322,10 +323,11 @@ impl Writeable for Output { /// an Output from a binary stream. impl Readable for Output { fn read(reader: &mut Reader) -> Result { - let (commit, proof) = ser_multiread!(reader, read_33_bytes, read_vec); + let commit = try!(Commitment::read(reader)); + let proof = try!(RangeProof::read(reader)); Ok(Output::BlindOutput { - commit: Commitment::from_vec(commit), - proof: RangeProof::from_vec(proof), + commit: commit, + proof: proof, }) } } diff --git a/core/src/genesis.rs b/core/src/genesis.rs index be6e92a79..01a95714c 100644 --- a/core/src/genesis.rs +++ b/core/src/genesis.rs @@ -18,17 +18,11 @@ use time; use core; use consensus::{DEFAULT_SIZESHIFT, MAX_TARGET}; - -use tiny_keccak::Keccak; +use core::hash::Hashed; // Genesis block definition. It has no rewards, no inputs, no outputs, no // fees and a height of zero. pub fn genesis() -> core::Block { - let mut sha3 = Keccak::new_sha3_256(); - let mut empty_h = [0; 32]; - sha3.update(&[]); - sha3.finalize(&mut empty_h); - core::Block { header: core::BlockHeader { height: 0, @@ -41,8 +35,8 @@ pub fn genesis() -> core::Block { }, cuckoo_len: DEFAULT_SIZESHIFT, target: MAX_TARGET, - utxo_merkle: core::hash::Hash::from_vec(empty_h.to_vec()), - tx_merkle: core::hash::Hash::from_vec(empty_h.to_vec()), + utxo_merkle: [].hash(), + tx_merkle: [].hash(), nonce: 0, pow: core::Proof::zero(), // TODO get actual PoW solution }, diff --git a/core/src/ser.rs b/core/src/ser.rs index 9240b7a74..3433137be 100644 --- a/core/src/ser.rs +++ b/core/src/ser.rs @@ -19,9 +19,13 @@ //! To use it simply implement `Writeable` or `Readable` and then use the //! `serialize` or `deserialize` functions on them as appropriate. -use std::{error, fmt}; +use std::{error, fmt, cmp}; use std::io::{self, Write, Read}; use byteorder::{ByteOrder, ReadBytesExt, BigEndian}; +use secp::pedersen::Commitment; +use secp::pedersen::RangeProof; +use secp::constants::PEDERSEN_COMMITMENT_SIZE; +use secp::constants::MAX_PROOF_SIZE; /// Possible errors deriving from serializing or deserializing. #[derive(Debug)] @@ -161,12 +165,10 @@ pub trait Reader { fn read_i64(&mut self) -> Result; /// first before the data bytes. fn read_vec(&mut self) -> Result, Error>; + /// first before the data bytes limited to max bytes. + fn read_limited_vec(&mut self, max: usize) -> Result, Error>; /// Read a fixed number of bytes from the underlying reader. fn read_fixed_bytes(&mut self, length: usize) -> Result, Error>; - /// Convenience function to read 32 fixed bytes - fn read_32_bytes(&mut self) -> Result, Error>; - /// Convenience function to read 33 fixed bytes - fn read_33_bytes(&mut self) -> Result, Error>; /// Consumes a byte from the reader, producing an error if it doesn't have /// the expected value fn expect_u8(&mut self, val: u8) -> Result; @@ -235,6 +237,11 @@ impl<'a> Reader for BinReader<'a> { let len = try!(self.read_u64()); self.read_fixed_bytes(len as usize) } + /// Read limited variable size vector from the underlying Read. Expects a usize + fn read_limited_vec(&mut self, max: usize) -> Result, Error> { + let len = cmp::min(max, try!(self.read_u64()) as usize); + self.read_fixed_bytes(len as usize) + } fn read_fixed_bytes(&mut self, length: usize) -> Result, Error> { // not reading more than 100k in a single read if length > 100000 { @@ -243,12 +250,7 @@ impl<'a> Reader for BinReader<'a> { let mut buf = vec![0; length]; self.source.read_exact(&mut buf).map(move |_| buf).map_err(Error::IOErr) } - fn read_32_bytes(&mut self) -> Result, Error> { - self.read_fixed_bytes(32) - } - fn read_33_bytes(&mut self) -> Result, Error> { - self.read_fixed_bytes(33) - } + fn expect_u8(&mut self, val: u8) -> Result { let b = try!(self.read_u8()); if b == val { @@ -262,6 +264,32 @@ impl<'a> Reader for BinReader<'a> { } } + +impl Readable for Commitment { + fn read(reader: &mut Reader) -> Result { + let a = try!(reader.read_fixed_bytes(PEDERSEN_COMMITMENT_SIZE)); + let mut c = [0; PEDERSEN_COMMITMENT_SIZE]; + for i in 0..PEDERSEN_COMMITMENT_SIZE { + c[i] = a[i]; + } + Ok(Commitment(c)) + } +} + +impl Readable for RangeProof { + fn read(reader: &mut Reader) -> Result { + let p = try!(reader.read_limited_vec(MAX_PROOF_SIZE)); + let mut a = [0; MAX_PROOF_SIZE]; + for i in 0..p.len() { + a[i] = p[i]; + } + Ok(RangeProof { + proof: a, + plen: p.len(), + }) + } +} + /// Utility wrapper for an underlying byte Writer. Defines higher level methods /// to write numbers, byte vectors, hashes, etc. struct BinWriter<'a> { diff --git a/secp256k1zkp/src/pedersen.rs b/secp256k1zkp/src/pedersen.rs index f554e68ad..9c84d31fd 100644 --- a/secp256k1zkp/src/pedersen.rs +++ b/secp256k1zkp/src/pedersen.rs @@ -28,7 +28,7 @@ use key::{SecretKey, PublicKey}; use rand::{Rng, OsRng}; /// A Pedersen commitment -pub struct Commitment([u8; constants::PEDERSEN_COMMITMENT_SIZE]); +pub struct Commitment(pub [u8; constants::PEDERSEN_COMMITMENT_SIZE]); impl_array_newtype!(Commitment, u8, constants::PEDERSEN_COMMITMENT_SIZE); impl_pretty_debug!(Commitment); @@ -37,14 +37,6 @@ impl Commitment { unsafe fn blank() -> Commitment { mem::uninitialized() } - /// Builds a commitment from a binary vector. Will panic if the vector is too short. - pub fn from_vec(c: Vec) -> Commitment { - let mut a = [0; constants::PEDERSEN_COMMITMENT_SIZE]; - for i in 0..a.len() { - a[i] = c[i]; - } - Commitment(a) - } /// Converts a commitment to a public key pub fn to_pubkey(&self, secp: &Secp256k1) -> Result { key::PublicKey::from_slice(secp, &self.0) @@ -83,17 +75,6 @@ impl Clone for RangeProof { } impl RangeProof { - /// Builds a range proof from a binary vector. Will panic if the vector is longer than the max proof size. - pub fn from_vec(p: Vec) -> RangeProof { - let mut a = [0; constants::MAX_PROOF_SIZE]; - for i in 0..p.len() { - a[i] = p[i]; - } - RangeProof { - proof: a, - plen: p.len(), - } - } pub fn bytes(&self) -> &[u8] { &self.proof[..self.plen as usize] }