fix(Rosetta): DEFI-2902: Enforce a 24h ingress window in Rosetta /construction/payloads

rs/rosetta-api/icp/src/request_handler/construction_parse.rs

@@ -787,8 +787,12 @@ mod tests {
             .duration_since(SystemTime::UNIX_EPOCH)
             .expect("Could not get system time")
             .as_nanos() as u64;
+        // `ingress_start` is kept within a realistic window around `now`: the
+        // ingress window is now bounded to 24h (ICPBB-134 / DEFI-2902), and this
+        // also avoids overflowing `ingress_start + ingress_interval` below.
+        let gen_ingress_start = proptest::option::of(now..(now + ONE_HOUR_NANOS));
         let gen_metadata = proptest::option::of(
-            (gen_opt_u64.clone(), gen_opt_u64.clone(), gen_opt_u64).prop_flat_map(
+            (gen_opt_u64.clone(), gen_ingress_start, gen_opt_u64).prop_flat_map(
                 |(created_at_time, ingress_start, memo)| {
                     proptest::option::of(1..ONE_HOUR_NANOS).prop_map(move |ingress_interval| {
                         let ingress_end = ingress_interval.map(|ingress_interval| {

rs/rosetta-api/icp/src/request_handler/construction_payloads.rs

@@ -34,6 +34,51 @@ use rosetta_core::convert::principal_id_from_public_key;
 use std::{collections::HashMap, sync::Arc, time::Duration};
 use tracing::log::debug;
 
+/// Maximum permitted ingress window, mirroring the 24-hour bound documented on
+/// `ConstructionPayloadsRequestMetadata`.
+const MAX_INGRESS_WINDOW: Duration = Duration::from_secs(24 * 60 * 60);
+
+/// Validate the caller-provided ingress window before it is expanded into a
+/// list of ingress expiries.
+///
+/// Rejects windows that would make the expiry loop iterate an excessive (or, on
+/// arithmetic wraparound, unbounded) number of times (see ICPBB-134 /
+/// DEFI-2902). It enforces that:
+/// - `ingress_start` is strictly before `ingress_end` (an empty or reversed
+///   window would otherwise produce an empty/degenerate set of payloads),
+/// - the span `ingress_end - ingress_start` must not exceed 24h, and
+/// - `ingress_end` must not be more than 24h in the future, which also rejects
+///   the near-`u64::MAX` payloads that would otherwise wrap the loop counter.
+fn validate_ingress_window(
+    now: ic_types::time::Time,
+    ingress_start: ic_types::time::Time,
+    ingress_end: ic_types::time::Time,
+) -> Result<(), ApiError> {
+    let max_window = MAX_INGRESS_WINDOW.as_nanos() as u64;
+    let now = now.as_nanos_since_unix_epoch();
+    let start = ingress_start.as_nanos_since_unix_epoch();
+    let end = ingress_end.as_nanos_since_unix_epoch();
+
+    if start >= end {
+        return Err(ApiError::invalid_request(
+            "ingress_start must be strictly before ingress_end.",
+        ));
+    }
+    if end.saturating_sub(start) > max_window {
+        return Err(ApiError::invalid_request(format!(
+            "The ingress window (ingress_end - ingress_start) must not exceed {} hours.",
+            MAX_INGRESS_WINDOW.as_secs() / 3600
+        )));
+    }
+    if end.saturating_sub(now) > max_window {
+        return Err(ApiError::invalid_request(format!(
+            "ingress_end must not be more than {} hours in the future.",
+            MAX_INGRESS_WINDOW.as_secs() / 3600
+        )));
+    }
+    Ok(())
+}
+
 impl RosettaRequestHandler {
     /// Generate an Unsigned Transaction and Signing Payloads.
     /// See https://www.rosetta-api.org/docs/ConstructionApi.html#constructionpayloads
@@ -96,6 +141,8 @@ impl RosettaRequestHandler {
             .map(Memo)
             .unwrap_or_else(|| Memo(rand::thread_rng().r#gen()));
 
+        validate_ingress_window(ic_types::time::current_time(), ingress_start, ingress_end)?;
+
         let mut ingress_expiries = vec![];
         let mut now = ingress_start;
         while now < ingress_end {
@@ -1110,3 +1157,68 @@ fn neuron_subaccount(
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use ic_types::time::Time;
+
+    const HOUR_NANOS: u64 = 60 * 60 * 1_000_000_000;
+    // A realistic "now" (~2023) that is well away from the u64 boundary.
+    const NOW_NANOS: u64 = 1_700_000_000 * 1_000_000_000;
+
+    fn t(nanos: u64) -> Time {
+        Time::from_nanos_since_unix_epoch(nanos)
+    }
+
+    // Vector A (ICPBB-134): a window far larger than the documented 24h bound is
+    // rejected before the loop.
+    #[test]
+    fn oversized_ingress_window_is_rejected() {
+        assert!(
+            validate_ingress_window(t(NOW_NANOS), t(NOW_NANOS), t(NOW_NANOS + 48 * HOUR_NANOS))
+                .is_err()
+        );
+    }
+
+    // Vector A' (ICPBB-134): a tiny start with a near-`u64::MAX` end (an enormous
+    // span) is rejected.
+    #[test]
+    fn unbounded_ingress_span_is_rejected() {
+        assert!(validate_ingress_window(t(NOW_NANOS), t(0), t(u64::MAX)).is_err());
+    }
+
+    // Vector B (ICPBB-134): the near-`u64::MAX` wrap payload has a tiny span but a
+    // far-future end, so it is rejected by the future bound.
+    #[test]
+    fn near_u64_max_ingress_window_is_rejected() {
+        let start = t(u64::MAX - 50 * 1_000_000_000);
+        assert!(validate_ingress_window(t(NOW_NANOS), start, t(u64::MAX)).is_err());
+    }
+
+    // A reversed or empty window (ingress_end <= ingress_start) is rejected
+    // rather than silently producing an empty set of payloads.
+    #[test]
+    fn reversed_ingress_window_is_rejected() {
+        assert!(validate_ingress_window(t(NOW_NANOS), t(NOW_NANOS + 1), t(NOW_NANOS)).is_err());
+        assert!(validate_ingress_window(t(NOW_NANOS), t(NOW_NANOS), t(NOW_NANOS)).is_err());
+    }
+
+    // Negative control: a realistic window (5 minutes ahead of now) is accepted.
+    #[test]
+    fn valid_ingress_window_is_accepted() {
+        let end = t(NOW_NANOS + 5 * 60 * 1_000_000_000);
+        assert!(validate_ingress_window(t(NOW_NANOS), t(NOW_NANOS), end).is_ok());
+    }
+
+    // Boundary: a window of exactly 24h ending exactly 24h from now is accepted,
+    // one nanosecond more is rejected.
+    #[test]
+    fn ingress_window_boundary_is_inclusive() {
+        let max = MAX_INGRESS_WINDOW.as_nanos() as u64;
+        assert!(validate_ingress_window(t(NOW_NANOS), t(NOW_NANOS), t(NOW_NANOS + max)).is_ok());
+        assert!(
+            validate_ingress_window(t(NOW_NANOS), t(NOW_NANOS), t(NOW_NANOS + max + 1)).is_err()
+        );
+    }
+}

rs/rosetta-api/icrc1/src/common/constants.rs

@@ -17,5 +17,8 @@ pub const SPENDER_OPERATION_IDENTIFIER: u64 = 7;
 pub const FEE_COLLECTOR_OPERATION_IDENTIFIER: u64 = 8;
 pub const MAX_TRANSACTIONS_PER_SEARCH_TRANSACTIONS_REQUEST: u64 = 10000;
 pub const INGRESS_INTERVAL_OVERLAP: Duration = Duration::from_secs(120);
+/// Maximum permitted ingress window, mirroring the 24-hour bound documented on
+/// `ConstructionPayloadsRequestMetadata`.
+pub const MAX_INGRESS_WINDOW: Duration = Duration::from_secs(24 * 60 * 60);
 pub const STATUS_COMPLETED: &str = "COMPLETED";
 pub const MAX_BLOCKS_PER_QUERY_BLOCK_RANGE_REQUEST: u64 = 10000;

rs/rosetta-api/icrc1/src/construction_api/services.rs

@@ -7,7 +7,7 @@ use super::utils::{
     handle_construction_hash, handle_construction_parse, handle_construction_payloads,
     handle_construction_submit,
 };
-use crate::common::constants::INGRESS_INTERVAL_OVERLAP;
+use crate::common::constants::{INGRESS_INTERVAL_OVERLAP, MAX_INGRESS_WINDOW};
 use crate::common::types::Error;
 use candid::Principal;
 use ic_base_types::{CanisterId, PrincipalId};
@@ -108,6 +108,32 @@ pub fn construction_combine(
         .map_err(|err| Error::processing_construction_failed(&err))
 }
 
+/// Validate the caller-provided ingress window before it is expanded into a
+/// list of ingress expiries.
+///
+/// Rejects windows that would make the expiry loop iterate an excessive (or, on
+/// arithmetic wraparound, unbounded) number of times (see ICPBB-134 /
+/// DEFI-2902). Two independent bounds are enforced:
+/// - the span `ingress_end - ingress_start` must not exceed 24h, and
+/// - `ingress_end` must not be more than 24h in the future, which also rejects
+///   the near-`u64::MAX` payloads that would otherwise wrap the loop counter.
+fn validate_ingress_window(now: u64, ingress_start: u64, ingress_end: u64) -> Result<(), Error> {
+    let max_window = MAX_INGRESS_WINDOW.as_nanos() as u64;
+    if ingress_end.saturating_sub(ingress_start) > max_window {
+        return Err(Error::processing_construction_failed(&format!(
+            "The ingress window (ingress_end - ingress_start) must not exceed {} hours: Start: {ingress_start}, End: {ingress_end}",
+            MAX_INGRESS_WINDOW.as_secs() / 3600
+        )));
+    }
+    if ingress_end.saturating_sub(now) > max_window {
+        return Err(Error::processing_construction_failed(&format!(
+            "ingress_end must not be more than {} hours in the future: Current time: {now}, End: {ingress_end}",
+            MAX_INGRESS_WINDOW.as_secs() / 3600
+        )));
+    }
+    Ok(())
+}
+
 pub fn construction_payloads(
     operations: Vec<Operation>,
     metadata: Option<ConstructionPayloadsRequestMetadata>,
@@ -157,6 +183,8 @@ pub fn construction_payloads(
         )));
     }
 
+    validate_ingress_window(now, ingress_start, ingress_end)?;
+
     // Every ingress message sent to the IC has an expiry timestamp until which the signature associated with that message is valid
     // To support a longer overall timeframe than one interval, we can send multiple ingress messages with two signable contents each
     let mut ingress_expiries = vec![];
@@ -261,6 +289,53 @@ mod tests {
     const NUM_TEST_CASES: u32 = 100;
     const NUM_BLOCKS: usize = 1;
 
+    const HOUR_NANOS: u64 = 60 * 60 * 1_000_000_000;
+    // A realistic "now" (~2023) that is well away from the u64 boundary.
+    const NOW_NANOS: u64 = 1_700_000_000 * 1_000_000_000;
+
+    // Vector A (ICPBB-134): a window far larger than the documented 24h bound is
+    // rejected before the loop.
+    #[test]
+    fn oversized_ingress_window_is_rejected() {
+        assert!(
+            validate_ingress_window(NOW_NANOS, NOW_NANOS, NOW_NANOS + 48 * HOUR_NANOS).is_err()
+        );
+    }
+
+    // Vector A' (ICPBB-134): a tiny start with a near-`u64::MAX` end (an enormous
+    // span) is rejected.
+    #[test]
+    fn unbounded_ingress_span_is_rejected() {
+        assert!(validate_ingress_window(NOW_NANOS, 0, u64::MAX).is_err());
+    }
+
+    // Vector B (ICPBB-134): the near-`u64::MAX` wrap payload has a tiny span but a
+    // far-future end, so it is rejected by the future bound.
+    #[test]
+    fn near_u64_max_ingress_window_is_rejected() {
+        assert!(
+            validate_ingress_window(NOW_NANOS, u64::MAX - 50 * 1_000_000_000, u64::MAX).is_err()
+        );
+    }
+
+    // Negative control: a realistic window (5 minutes ahead of now) is accepted.
+    #[test]
+    fn valid_ingress_window_is_accepted() {
+        assert!(
+            validate_ingress_window(NOW_NANOS, NOW_NANOS, NOW_NANOS + 5 * 60 * 1_000_000_000)
+                .is_ok()
+        );
+    }
+
+    // Boundary: a window of exactly 24h ending exactly 24h from now is accepted,
+    // one nanosecond more is rejected.
+    #[test]
+    fn ingress_window_boundary_is_inclusive() {
+        let max = MAX_INGRESS_WINDOW.as_nanos() as u64;
+        assert!(validate_ingress_window(NOW_NANOS, NOW_NANOS, NOW_NANOS + max).is_ok());
+        assert!(validate_ingress_window(NOW_NANOS, NOW_NANOS, NOW_NANOS + max + 1).is_err());
+    }
+
     fn call_construction_derive<T: RosettaSupportedKeyPair>(key_pair: &T) {
         let principal_id = key_pair.generate_principal_id().unwrap();
         let public_key = ic_rosetta_test_utils::to_public_key(key_pair);
@@ -461,6 +536,17 @@ mod tests {
                             }
                             (_, _) => {}
                         }
+                        // Windows wider than the permitted maximum, or with an
+                        // ingress_end too far in the future, are rejected before
+                        // the loop (ICPBB-134 / DEFI-2902).
+                        let eff_start = payloads_metadata.ingress_start.unwrap_or(now);
+                        let eff_end = payloads_metadata
+                            .ingress_end
+                            .unwrap_or(eff_start + ingress_interval);
+                        if validate_ingress_window(now, eff_start, eff_end).is_err() {
+                            assert!(construction_payloads_response.is_err());
+                            continue;
+                        }
                         let construction_parse_response = construction_parse(
                             construction_payloads_response
                                 .clone()