From 76c655ae2d737dff95cd72ae97f247cc0385aafe Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 15 Sep 2026 17:03:16 +0000 Subject: [PATCH 1/2] fix(client): bound server-driven pagination in drain_cursor_pages drain_cursor_pages accumulated pages from an untrusted representative until the node returned an empty page or a None cursor - both server-controlled. With no cursor-progress check and no ceiling, a malicious node that always returns a non-empty page plus Some(next_key) (e.g. by replaying one page or echoing a constant cursor) could drive unbounded memory growth or a non-terminating loop in the client. Reached from the public chain_all/history_all/global_history_all. Require the cursor to strictly advance (tighten CURSOR to Copy + PartialEq) and add a MAX_DRAINED_ITEMS safety ceiling; both return the new ClientError::PaginationLimitExceeded instead of looping/allocating without bound. Co-authored-by: Ty Schenk --- keetanetwork-client/src/client.rs | 34 +++++++++++++++++++++++++++++-- keetanetwork-client/src/error.rs | 6 ++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/keetanetwork-client/src/client.rs b/keetanetwork-client/src/client.rs index 7884349..5c3774a 100644 --- a/keetanetwork-client/src/client.rs +++ b/keetanetwork-client/src/client.rs @@ -1972,9 +1972,16 @@ fn store_representatives(_runtime: &Arc, _signature: &str, _reps: & /// Drain a cursor-paged read: call `fetch` with no cursor, then with each /// page's `next_key`, until the node reports the end of the sequence. +/// +/// Safety ceiling on the total number of items a single cursor-paged read may +/// accumulate from an untrusted node before it is treated as abusive. This is a +/// backstop for the cursor-progress guard below; adjust or make configurable if +/// a legitimate sequence can exceed it. +const MAX_DRAINED_ITEMS: usize = 1_000_000; + async fn drain_cursor_pages(fetch: FETCH) -> Result, ClientError> where - CURSOR: Copy, + CURSOR: Copy + PartialEq, FETCH: Fn(Option) -> PAGE, PAGE: Future, Option), ClientError>>, { @@ -1990,8 +1997,20 @@ where items.extend(page); + // The remote node controls both the page contents and the `next_key` + // cursor. Without these guards a malicious or buggy node could return a + // non-empty page plus a cursor forever (e.g. by replaying one page or + // echoing a constant cursor), driving unbounded memory growth or a + // non-terminating loop. Require the cursor to advance and cap the total + // number of accumulated items as a safety net. + if items.len() > MAX_DRAINED_ITEMS { + return Err(ClientError::PaginationLimitExceeded); + } + match next_key { - Some(next) => cursor = Some(next), + Some(next) if Some(next) != cursor => cursor = Some(next), + // End of sequence (`None`) or a cursor that did not advance: stop. + Some(_) => return Err(ClientError::PaginationLimitExceeded), None => break, } } @@ -2167,6 +2186,17 @@ mod tests { Ok(()) } + #[test] + fn drain_cursor_pages_rejects_a_non_advancing_cursor() -> TestResult { + // A malicious node that always returns a non-empty page plus the same + // (non-advancing) cursor must be rejected rather than looped forever. + let fetch = |_: Option| core::future::ready(Ok((alloc::vec![1u8], Some(7u8)))); + + let result = resolve(drain_cursor_pages(fetch)).ok_or("ready future")?; + assert!(matches!(result, Err(ClientError::PaginationLimitExceeded))); + Ok(()) + } + #[test] fn drain_cursor_pages_stops_when_the_cursor_runs_out() -> TestResult { let calls = core::cell::Cell::new(0u32); diff --git a/keetanetwork-client/src/error.rs b/keetanetwork-client/src/error.rs index 95db6be..9f5e910 100644 --- a/keetanetwork-client/src/error.rs +++ b/keetanetwork-client/src/error.rs @@ -141,6 +141,11 @@ pub enum ClientError { #[snafu(display("no representatives available"))] NoRepresentatives, + /// A cursor-paged read exceeded its safety ceiling or the node returned a + /// non-advancing cursor, indicating an unbounded or looping response. + #[snafu(display("paginated response exceeded its safety limit or did not make progress"))] + PaginationLimitExceeded, + /// A request exceeded the configured per-request timeout. #[snafu(display("request timed out"))] Timeout, @@ -225,6 +230,7 @@ impl ClientError { Self::Account { .. } => "ACCOUNT", Self::UnsupportedNetwork => "UNSUPPORTED_NETWORK", Self::NoRepresentatives => "NO_REPRESENTATIVES", + Self::PaginationLimitExceeded => "PAGINATION_LIMIT_EXCEEDED", Self::Timeout => "TIMEOUT", Self::QuorumNotReached => "QUORUM_NOT_REACHED", Self::SyncPublishFailed => "SYNC_PUBLISH_FAILED", From fc22422b3e8cbd05932ed71530a67e0bda940933 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 16 Sep 2026 15:45:18 +0000 Subject: [PATCH 2/2] test(client): cover MAX_DRAINED_ITEMS ceiling Co-authored-by: Ty Schenk --- keetanetwork-client/src/client.rs | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/keetanetwork-client/src/client.rs b/keetanetwork-client/src/client.rs index 5c3774a..85dc308 100644 --- a/keetanetwork-client/src/client.rs +++ b/keetanetwork-client/src/client.rs @@ -2009,7 +2009,8 @@ where match next_key { Some(next) if Some(next) != cursor => cursor = Some(next), - // End of sequence (`None`) or a cursor that did not advance: stop. + // Repeated cursor: the node did not advance, so reject rather than + // loop on a replay. `None` below is the legitimate end of sequence. Some(_) => return Err(ClientError::PaginationLimitExceeded), None => break, } @@ -2197,6 +2198,27 @@ mod tests { Ok(()) } + #[test] + fn drain_cursor_pages_rejects_when_advancing_pages_exceed_the_item_ceiling() -> TestResult { + // A node that keeps returning non-empty pages with strictly advancing + // cursors must still be rejected once the aggregate item ceiling is + // crossed; cursor progress alone is not enough. + let calls = core::cell::Cell::new(0u32); + let page_len = MAX_DRAINED_ITEMS / 4 + 1; + let fetch = |_: Option| { + let call = calls.get(); + calls.set(call + 1); + core::future::ready(Ok((alloc::vec![(); page_len], Some(call)))) + }; + + let result = resolve(drain_cursor_pages(fetch)).ok_or("ready future")?; + assert!(matches!(result, Err(ClientError::PaginationLimitExceeded))); + // Four advancing pages cross the ceiling; a non-advancing cursor would + // have failed on the second call. + assert_eq!(calls.get(), 4); + Ok(()) + } + #[test] fn drain_cursor_pages_stops_when_the_cursor_runs_out() -> TestResult { let calls = core::cell::Cell::new(0u32);