Merge pull request #22 from datachainlab/audit-202411-eelc-2

EELC2: improve validation of `Fraction`

Signed-off-by: Jun Kimura <[email protected]>

.github/workflows/test.yml

@@ -22,6 +22,8 @@ jobs:
     steps:
       - uses: actions/checkout@v4
       - uses: actions-rust-lang/setup-rust-toolchain@v1
+        with:
+          toolchain: 1.82.0
       - uses: Swatinem/rust-cache@v2
       - run: cargo test
       - run: cargo build

crates/light-client-cli/src/client.rs

@@ -70,7 +70,8 @@ impl<
             verifier: Default::default(),
             genesis_time,
             genesis_validators_root,
-            trust_level: trust_level.unwrap_or(Fraction::new(2, 3)),
+            // safe to unwrap: `2/3` is valid fraction
+            trust_level: trust_level.unwrap_or(Fraction::new(2, 3).unwrap()),
         }
     }
 

crates/light-client-verifier/src/consensus.rs

@@ -194,9 +194,9 @@ pub fn verify_sync_committee_attestation<
             participants,
             ctx.min_sync_committee_participants(),
         ));
-    } else if participants as u64 * ctx.signature_threshold().denominator
+    } else if participants as u64 * ctx.signature_threshold().denominator()
         < consensus_update.sync_aggregate().sync_committee_bits.len() as u64
-            * ctx.signature_threshold().numerator
+            * ctx.signature_threshold().numerator()
     {
         return Err(Error::InsufficientParticipants(
             participants as u64,
@@ -881,7 +881,7 @@ mod tests {
                 genesis_validators_root,
                 // NOTE: this is workaround. we must get the correct timestamp from beacon state.
                 minimal::get_config().min_genesis_time,
-                Fraction::new(2, 3),
+                Fraction::new(2, 3).unwrap(),
                 1729846322.into(),
             );
             assert!(verifier.validate_boostrap(&ctx, &bootstrap, None).is_ok());
@@ -928,7 +928,7 @@ mod tests {
                 genesis_validators_root,
                 // NOTE: this is workaround. we must get the correct timestamp from beacon state.
                 minimal::get_config().min_genesis_time,
-                Fraction::new(2, 3),
+                Fraction::new(2, 3).unwrap(),
                 1729846322.into(),
             );
             assert!(verifier.validate_boostrap(&ctx, &bootstrap, None).is_ok());
@@ -1039,7 +1039,7 @@ mod tests {
                 config::minimal::get_config(),
                 Default::default(),
                 Default::default(),
-                Fraction::new(2, 3),
+                Fraction::new(2, 3).unwrap(),
                 SystemTime::now()
                     .duration_since(SystemTime::UNIX_EPOCH)
                     .unwrap()
@@ -1458,7 +1458,7 @@ mod tests {
                 config::minimal::get_config(),
                 Default::default(),
                 Default::default(),
-                Fraction::new(2, 3),
+                Fraction::new(2, 3).unwrap(),
                 SystemTime::now()
                     .duration_since(SystemTime::UNIX_EPOCH)
                     .unwrap()
@@ -1779,7 +1779,7 @@ mod tests {
                         .as_ref(),
                 ),
                 1731420304.into(),
-                Fraction::new(2, 3),
+                Fraction::new(2, 3).unwrap(),
                 SystemTime::now()
                     .duration_since(SystemTime::UNIX_EPOCH)
                     .unwrap()
@@ -1806,7 +1806,7 @@ mod tests {
                         .as_ref(),
                 ),
                 1731420304.into(),
-                Fraction::new(2, 3),
+                Fraction::new(2, 3).unwrap(),
                 SystemTime::now()
                     .duration_since(SystemTime::UNIX_EPOCH)
                     .unwrap()
@@ -1971,7 +1971,7 @@ mod tests {
                         .as_ref(),
                 ),
                 1731420304.into(),
-                Fraction::new(2, 3),
+                Fraction::new(2, 3).unwrap(),
                 SystemTime::now()
                     .duration_since(SystemTime::UNIX_EPOCH)
                     .unwrap()

crates/light-client-verifier/src/context.rs

@@ -1,3 +1,4 @@
+use crate::errors::Error;
 use ethereum_consensus::{
     beacon::{Epoch, Root, Slot},
     compute::{compute_epoch_at_slot, compute_slot_at_timestamp},
@@ -6,19 +7,36 @@ use ethereum_consensus::{
     fork::{ForkParameters, ForkSpec},
     types::U64,
 };
+
+/// Fraction is a struct representing a fraction for a threshold.
 #[derive(Clone, Default, Debug, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
 pub struct Fraction {
-    pub numerator: u64,
-    pub denominator: u64,
+    numerator: u64,
+    denominator: u64,
 }
 
 impl Fraction {
-    pub fn new(numerator: u64, denominator: u64) -> Self {
-        Self {
-            numerator,
-            denominator,
+    pub fn new(numerator: u64, denominator: u64) -> Result<Self, Error> {
+        if denominator == 0 || numerator > denominator {
+            Err(Error::InvalidFraction(Self {
+                numerator,
+                denominator,
+            }))
+        } else {
+            Ok(Self {
+                numerator,
+                denominator,
+            })
         }
     }
+
+    pub fn numerator(&self) -> u64 {
+        self.numerator
+    }
+
+    pub fn denominator(&self) -> u64 {
+        self.denominator
+    }
 }
 
 pub trait ConsensusVerificationContext {

crates/light-client-verifier/src/errors.rs

@@ -1,4 +1,4 @@
-use crate::internal_prelude::*;
+use crate::{context::Fraction, internal_prelude::*};
 use displaydoc::Display;
 use ethereum_consensus::{
     beacon::{BeaconBlockHeader, Epoch, Root, Slot},
@@ -91,6 +91,8 @@ pub enum Error {
     InvalidExecutionBlockNumberMerkleBranch(MerkleError),
     /// inconsistent next sync committee: `store:{0:?}` != `update:{1:?}`
     InconsistentNextSyncCommittee(PublicKey, PublicKey),
+    /// invalid fraction: `fraction={0:?}`
+    InvalidFraction(Fraction),
     /// other error: `{description}`
     Other { description: String },
 }