Merge pull request #151 from rust-bitcoin/2018-08-serialization-fixes

Various serialization fixes

fuzz/fuzz_targets/deserialize_script.rs

@@ -31,6 +31,7 @@ fn do_test(data: &[u8]) {
             }
         }
         assert_eq!(b.into_script(), script);
+        assert_eq!(data, &serialize::serialize(&script).unwrap()[..]);
     }
 }
 

src/blockdata/transaction.rs

@@ -408,12 +408,16 @@ impl<D: SimpleDecoder> ConsensusDecodable<D> for Transaction {
                     for txin in input.iter_mut() {
                         txin.witness = ConsensusDecodable::consensus_decode(d)?;
                     }
-                    Ok(Transaction {
+                    if !input.is_empty() && input.iter().all(|input| input.witness.is_empty()) {
-                        version: version,
+                        Err(serialize::Error::ParseFailed("witness flag set but no witnesses present"))
-                        input: input,
+                    } else {
-                        output: output,
+                        Ok(Transaction {
-                        lock_time: ConsensusDecodable::consensus_decode(d)?,
+                            version: version,
-                    })
+                            input: input,
+                            output: output,
+                            lock_time: ConsensusDecodable::consensus_decode(d)?,
+                        })
+                    }
                 }
                 // We don't support anything else
                 x => {

src/network/encodable.rs

@@ -115,14 +115,36 @@ impl<D: SimpleDecoder> ConsensusDecodable<D> for VarInt {
     fn consensus_decode(d: &mut D) -> Result<VarInt, serialize::Error> {
         let n = d.read_u8()?;
         match n {
-            0xFF => d.read_u64().map(|n| VarInt(u64::from_le(n))),
+            0xFF => {
-            0xFE => d.read_u32().map(|n| VarInt(u32::from_le(n) as u64)),
+                let x = d.read_u64()?;
-            0xFD => d.read_u16().map(|n| VarInt(u16::from_le(n) as u64)),
+                if x < 0x100000000 {
+                    Err(serialize::Error::ParseFailed("non-minimal varint"))
+                } else {
+                    Ok(VarInt(x))
+                }
+            }
+            0xFE => {
+                let x = d.read_u32()?;
+                if x < 0x10000 {
+                    Err(serialize::Error::ParseFailed("non-minimal varint"))
+                } else {
+                    Ok(VarInt(x as u64))
+                }
+            }
+            0xFD => {
+                let x = d.read_u16()?;
+                if x < 0xFD {
+                    Err(serialize::Error::ParseFailed("non-minimal varint"))
+                } else {
+                    Ok(VarInt(x as u64))
+                }
+            }
             n => Ok(VarInt(n as u64))
         }
     }
 }
 
+
 // Booleans
 impl<S: SimpleEncoder> ConsensusEncodable<S> for bool {
     #[inline]
@@ -202,7 +224,7 @@ impl<S: SimpleEncoder, T: ConsensusEncodable<S>> ConsensusEncodable<S> for Vec<T
 impl<D: SimpleDecoder, T: ConsensusDecodable<D>> ConsensusDecodable<D> for Vec<T> {
     #[inline]
     fn consensus_decode(d: &mut D) -> Result<Vec<T>, serialize::Error> {
-        let VarInt(len): VarInt = ConsensusDecodable::consensus_decode(d)?;
+        let len = VarInt::consensus_decode(d)?.0;
         let byte_size = (len as usize)
                             .checked_mul(mem::size_of::<T>())
                             .ok_or(serialize::Error::ParseFailed("Invalid length"))?;
@@ -223,7 +245,7 @@ impl<S: SimpleEncoder, T: ConsensusEncodable<S>> ConsensusEncodable<S> for Box<[
 impl<D: SimpleDecoder, T: ConsensusDecodable<D>> ConsensusDecodable<D> for Box<[T]> {
     #[inline]
     fn consensus_decode(d: &mut D) -> Result<Box<[T]>, serialize::Error> {
-        let VarInt(len): VarInt = ConsensusDecodable::consensus_decode(d)?;
+        let len = VarInt::consensus_decode(d)?.0;
         let len = len as usize;
         if len > MAX_VEC_SIZE {
             return Err(serialize::Error::OversizedVectorAllocation { requested: len, max: MAX_VEC_SIZE })
@@ -366,7 +388,7 @@ impl<D, K, V> ConsensusDecodable<D> for HashMap<K, V>
 {
     #[inline]
     fn consensus_decode(d: &mut D) -> Result<HashMap<K, V>, serialize::Error> {
-        let VarInt(len): VarInt = ConsensusDecodable::consensus_decode(d)?;
+        let len = VarInt::consensus_decode(d)?.0;
 
         let mut ret = HashMap::with_capacity(len as usize);
         for _ in 0..len {
@@ -384,7 +406,7 @@ impl<D, K, V> ConsensusDecodable<D> for HashMap<K, V>
 mod tests {
     use super::{CheckedData, VarInt};
 
-    use network::serialize::{deserialize, serialize};
+    use network::serialize::{deserialize, serialize, Error};
 
     #[test]
     fn serialize_int_test() {
@@ -435,6 +457,46 @@ mod tests {
         assert_eq!(serialize(&VarInt(0xF0F0F0F0F0E0)).ok(), Some(vec![0xFFu8, 0xE0, 0xF0, 0xF0, 0xF0, 0xF0, 0xF0, 0, 0]));
     }
 
+    #[test]
+    fn deserialize_nonminimal_vec() {
+        match deserialize::<Vec<u8>>(&[0xfd, 0x00, 0x00]) {
+            Err(Error::ParseFailed("non-minimal varint")) => {},
+            x => panic!(x)
+        }
+        match deserialize::<Vec<u8>>(&[0xfd, 0xfc, 0x00]) {
+            Err(Error::ParseFailed("non-minimal varint")) => {},
+            x => panic!(x)
+        }
+        match deserialize::<Vec<u8>>(&[0xfe, 0xff, 0x00, 0x00, 0x00]) {
+            Err(Error::ParseFailed("non-minimal varint")) => {},
+            x => panic!(x)
+        }
+        match deserialize::<Vec<u8>>(&[0xfe, 0xff, 0xff, 0x00, 0x00]) {
+            Err(Error::ParseFailed("non-minimal varint")) => {},
+            x => panic!(x)
+        }
+        match deserialize::<Vec<u8>>(&[0xff, 0xff, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00]) {
+            Err(Error::ParseFailed("non-minimal varint")) => {},
+            x => panic!(x)
+        }
+        match deserialize::<Vec<u8>>(&[0xff, 0xff, 0xff, 0xff, 0x00, 0x00, 0x00, 0x00, 0x00]) {
+            Err(Error::ParseFailed("non-minimal varint")) => {},
+            x => panic!(x)
+        }
+
+        let mut vec_256 = vec![0; 259];
+        vec_256[0] = 0xfd;
+        vec_256[1] = 0x00;
+        vec_256[2] = 0x01;
+        assert!(deserialize::<Vec<u8>>(&vec_256).is_ok());
+
+        let mut vec_253 = vec![0; 256];
+        vec_253[0] = 0xfd;
+        vec_253[1] = 0xfd;
+        vec_253[2] = 0x00;
+        assert!(deserialize::<Vec<u8>>(&vec_253).is_ok());
+    }
+
     #[test]
     fn serialize_checkeddata_test() {
         let cd = CheckedData(vec![1u8, 2, 3, 4, 5]);