[Audit fixes] FOR-03 - Inconsistent Deserialization of Randomness

blockchain/chain_sync/src/tipset_syncer.rs

@@ -1580,7 +1580,7 @@ fn verify_winning_post_proof<DB: BlockStore + Send + Sync + 'static, V: ProofVer
             &lookback_state,
             network_version,
             &header.miner_address(),
-            Randomness(rand),
+            Randomness(rand.to_vec()),
         )
         .map_err(|e| {
             TipsetRangeSyncerError::Validation(format!(
@@ -1589,14 +1589,18 @@ fn verify_winning_post_proof<DB: BlockStore + Send + Sync + 'static, V: ProofVer
             ))
         })?;
 
-    V::verify_winning_post(Randomness(rand), header.winning_post_proof(), &sectors, id).map_err(
-        |e| {
-            TipsetRangeSyncerError::Validation(format!(
-                "Failed to verify winning PoSt: {}",
-                e.to_string()
-            ))
-        },
+    V::verify_winning_post(
+        Randomness(rand.to_vec()),
+        header.winning_post_proof(),
+        &sectors,
+        id,
     )
+    .map_err(|e| {
+        TipsetRangeSyncerError::Validation(format!(
+            "Failed to verify winning PoSt: {}",
+            e.to_string()
+        ))
+    })
 }
 
 fn check_block_messages<

blockchain/state_manager/src/lib.rs

@@ -669,8 +669,12 @@ where
         )?;
 
         let nv = get_network_version_default(tipset.epoch());
-        let sectors =
-            self.get_sectors_for_winning_post::<V>(&lbst, nv, &address, Randomness(prand))?;
+        let sectors = self.get_sectors_for_winning_post::<V>(
+            &lbst,
+            nv,
+            &address,
+            Randomness(prand.to_vec()),
+        )?;
 
         if sectors.is_empty() {
             return Ok(None);

encoding/src/bytes.rs

@@ -41,6 +41,12 @@ impl<'de> Deserialize<'de> for Byte32De {
     }
 }
 
+pub fn bytes_32(buf: &Vec<u8>) -> [u8; 32] {
+    let mut array = [0; 32];
+    array.copy_from_slice(buf.as_ref());
+    array
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

types/src/randomness.rs

@@ -1,12 +1,14 @@
 // Copyright 2020 ChainSafe Systems
 // SPDX-License-Identifier: Apache-2.0, MIT
 
-use encoding::{Byte32De, BytesSer};
+use encoding::{BytesDe, BytesSer};
 use serde::{Deserialize, Deserializer, Serialize, Serializer};
 
 /// String of random bytes usually generated from a randomness beacon or from tickets on chain.
-#[derive(PartialEq, Eq, Default, Copy, Clone, Debug)]
-pub struct Randomness(pub [u8; 32]);
+#[derive(PartialEq, Eq, Default, Clone, Debug)]
+pub struct Randomness(pub Vec<u8>);
+
+pub const RANDOMNESS_LENGTH: usize = 32;
 
 impl Serialize for Randomness {
     fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
@@ -22,7 +24,7 @@ impl<'de> Deserialize<'de> for Randomness {
     where
         D: Deserializer<'de>,
     {
-        let bytes = Byte32De::deserialize(deserializer)?;
+        let bytes = BytesDe::deserialize(deserializer)?;
         Ok(Self(bytes.0))
     }
 }

types/src/verifier/mod.rs

@@ -11,6 +11,7 @@ use crate::{
 };
 use address::Address;
 use commcid::{cid_to_data_commitment_v1, cid_to_replica_commitment_v1};
+use encoding::bytes_32;
 use filecoin_proofs_api::{self as proofs, ProverId, SectorId};
 use filecoin_proofs_api::{
     post, seal::verify_aggregate_seal_commit_proofs, seal::verify_seal as proofs_verify_seal,
@@ -43,8 +44,8 @@ pub trait ProofVerifier {
             commd,
             prover_id,
             SectorId::from(vi.sector_id.number),
-            vi.randomness.0,
-            vi.interactive_randomness.0,
+            bytes_32(&vi.randomness.0),
+            bytes_32(&vi.interactive_randomness.0),
             &vi.proof,
         )? {
             Err("Invalid Seal proof".into())
@@ -81,8 +82,8 @@ pub trait ProofVerifier {
                     commr,
                     commd,
                     sector_id: info.sector_number,
-                    ticket: info.randomness.0,
-                    seed: info.interactive_randomness.0,
+                    ticket: bytes_32(&info.randomness.0),
+                    seed: bytes_32(&info.interactive_randomness.0),
                 })
             })
             .collect::<Result<Vec<_>, &'static str>>()?;
@@ -144,7 +145,7 @@ pub trait ProofVerifier {
         let prover_id = prover_id_from_u64(prover);
 
         // Verify Proof
-        if !post::verify_winning_post(&randomness, &proof_bytes, &replicas, prover_id)? {
+        if !post::verify_winning_post(&bytes_32(&randomness), &proof_bytes, &replicas, prover_id)? {
             Err("Winning post was invalid".into())
         } else {
             Ok(())
@@ -176,7 +177,7 @@ pub trait ProofVerifier {
         let prover_id = prover_id_from_u64(prover);
 
         // Verify Proof
-        if !post::verify_window_post(&randomness, &proofs, &replicas, prover_id)? {
+        if !post::verify_window_post(&bytes_32(&randomness), &proofs, &replicas, prover_id)? {
             Err("Window post was invalid".into())
         } else {
             Ok(())
@@ -195,7 +196,7 @@ pub trait ProofVerifier {
 
         Ok(post::generate_winning_post_sector_challenge(
             proof.try_into()?,
-            &randomness,
+            &bytes_32(&randomness),
             eligible_sector_count,
             prover_id_from_u64(prover_id),
         )?)

vm/actor/src/builtin/miner/mod.rs

@@ -70,6 +70,7 @@ use fil_types::{
     deadlines::DeadlineInfo, InteractiveSealRandomness, PoStProof, PoStRandomness,
     RegisteredSealProof, SealRandomness as SealRandom, SealVerifyInfo, SealVerifyParams, SectorID,
     SectorInfo, SectorNumber, SectorSize, WindowPoStVerifyInfo, MAX_SECTOR_NUMBER,
+    RANDOMNESS_LENGTH,
 };
 use ipld_blockstore::BlockStore;
 use num_bigint::bigint_ser::BigIntSer;
@@ -420,16 +421,14 @@ impl Actor {
             ));
         }
 
-        // * This check is invalid because our randomness length is always == 32
-        // * and there is no clear need for less randomness
-        // if params.chain_commit_rand.0.len() > RANDOMNESS_LENGTH {
-        //     return Err(actor_error!(
-        //         ErrIllegalArgument,
-        //         "expected at most {} bytes of randomness, got {}",
-        //         RANDOMNESS_LENGTH,
-        //         params.chain_commit_rand.0.len()
-        //     ));
-        // }
+        if params.chain_commit_rand.0.len() > RANDOMNESS_LENGTH {
+            return Err(actor_error!(
+                ErrIllegalArgument,
+                "expected at most {} bytes of randomness, got {}",
+                RANDOMNESS_LENGTH,
+                params.chain_commit_rand.0.len()
+            ));
+        }
 
         let post_result = rt.transaction(|state: &mut State, rt| {
             let info = get_miner_info(rt.store(), state)?;

vm/interpreter/src/default_runtime.rs

@@ -625,7 +625,7 @@ where
                 .map_err(|e| e.downcast_fatal("could not get randomness"))?
         };
 
-        Ok(Randomness(r))
+        Ok(Randomness(r.to_vec()))
     }
 
     fn get_randomness_from_beacon(
@@ -643,7 +643,7 @@ where
                 .get_beacon_randomness(personalization, rand_epoch, entropy)
                 .map_err(|e| e.downcast_fatal("could not get randomness"))?
         };
-        Ok(Randomness(r))
+        Ok(Randomness(r.to_vec()))
     }
 
     fn create<C: Cbor>(&mut self, obj: &C) -> Result<(), ActorError> {
@@ -893,7 +893,12 @@ where
             .borrow_mut()
             .charge_gas(self.price_list.on_verify_post(vi))?;
 
-        V::verify_window_post(vi.randomness, &vi.proofs, &vi.challenged_sectors, vi.prover)
+        V::verify_window_post(
+            vi.randomness.clone(),
+            &vi.proofs,
+            &vi.challenged_sectors,
+            vi.prover,
+        )
     }
     fn verify_consensus_fault(
         &self,