fix(sdk): bind document query verification to server-served wire versions and limits

Two trust-boundary gaps let an untrusted transport pair a valid proof with a request the supplied platform version's server would have refused. First, the wire version oneof (V0/V1) was never checked against platform_version.drive_abci.query.document_query bounds, so a V1 request could be verified under a platform version whose server only serves V0. Both verify entry points now run the same check_version gate the server's query_documents dispatch runs, before any decoding, contract lookup, or proof machinery.

Second, the DocumentQuery -> DriveDocumentQuery lowering only guarded the u16 cast, so limits 101..=65535 - which the server refuses with InvalidLimit via DriveDocumentQuery::from_typed_clauses' default_query_limit cap (100) - reached a raw DriveDocumentQuery and could verify a proof no honest server would produce. The lowering now mirrors from_typed_clauses exactly: 0 stays the unset-default sentinel, 1..=100 passes, anything above is refused with the server's own QuerySyntaxError::InvalidLimit.

Tests reuse the panicking ContextProvider to pin that both rejections happen before any proof machinery runs, and cover the provider-resolved entry point rejecting before its contract lookup.

Author: PastaPastaPasta

packages/dash-platform-queries/src/documents/document_query.rs

@@ -31,6 +31,7 @@ use dpp::{
     prelude::{DataContract, Identifier},
     InvalidVectorSizeError, ProtocolError,
 };
+use drive::config::DEFAULT_QUERY_LIMIT;
 use drive::query::drive_document_ranked_query::mode_detection::ranked_order_key;
 use drive::query::{
     DriveDocumentQuery, HavingAggregate, HavingAggregateFunction, HavingClause, HavingOperator,
@@ -651,6 +652,46 @@ fn order_clauses_from_cbor(bytes: &[u8]) -> Result<Vec<OrderClause>, Error> {
     }
 }
 
+/// Reject a request whose wire version (the `V0`/`V1` oneof arm,
+/// i.e. feature version 0/1) falls outside the supplied platform
+/// version's `drive_abci.query.document_query` bounds.
+///
+/// This is the same `check_version` gate the server runs before it
+/// decodes anything (`Platform::query_documents` in rs-drive-abci,
+/// which answers an out-of-bounds wire version with
+/// `QueryError::UnsupportedQueryVersion`). Without it, an untrusted
+/// transport could pair a request wire-version the supplied platform
+/// version's server refuses to serve with a valid proof produced for
+/// the other wire shape, and verification would accept the pair.
+///
+/// A missing `version` oneof is deliberately let through — the decode
+/// that follows reports it with its established error message.
+fn check_wire_version_is_served(
+    request: &GetDocumentsRequest,
+    platform_version: &PlatformVersion,
+) -> Result<(), drive_proof_verifier::Error> {
+    let Some(version) = &request.version else {
+        return Ok(());
+    };
+    let feature_version: u16 = match version {
+        V0(_) => 0,
+        V1(_) => 1,
+    };
+    let bounds = &platform_version.drive_abci.query.document_query;
+    if !bounds.check_version(feature_version) {
+        return Err(drive_proof_verifier::Error::RequestError {
+            error: format!(
+                "GetDocumentsRequest wire version V{feature_version} is outside the \
+                 document_query feature-version bounds {}..={} served at platform version \
+                 {}; the server answers such a request with UnsupportedQueryVersion, so no \
+                 proved response can belong to it",
+                bounds.min_version, bounds.max_version, platform_version.protocol_version
+            ),
+        });
+    }
+    Ok(())
+}
+
 /// Reject the request shapes that can never have produced the proved
 /// plain-document response being verified.
 ///
@@ -748,6 +789,17 @@ fn reject_request_the_server_would_not_have_proved(
 /// front, mirroring the server's own gates in
 /// `rs-drive-abci`'s `validate_and_route` /
 /// `reject_offset_off_the_ranked_path`.
+///
+/// The same reasoning covers the request envelope itself: the wire
+/// version (`V0`/`V1` oneof arm) is checked against
+/// `platform_version.drive_abci.query.document_query`'s bounds before
+/// anything is decoded — the server's `query_documents` dispatch
+/// refuses an out-of-bounds wire version with
+/// `UnsupportedQueryVersion`, so a proof can never belong to one — and
+/// the query limit is capped at the server's
+/// [`DEFAULT_QUERY_LIMIT`] during the `DriveDocumentQuery` lowering,
+/// exactly as `DriveDocumentQuery::from_typed_clauses` caps it
+/// server-side.
 pub fn verify_documents_response(
     request: GetDocumentsRequest,
     contract: Arc<DataContract>,
@@ -756,6 +808,10 @@ pub fn verify_documents_response(
     platform_version: &PlatformVersion,
     provider: &dyn ContextProvider,
 ) -> Result<(Option<Documents>, ResponseMetadata, Proof), drive_proof_verifier::Error> {
+    // First gate, mirroring the server's own dispatch order: a wire
+    // version the supplied platform version's server refuses to serve
+    // is rejected before any decoding or proof machinery.
+    check_wire_version_is_served(&request, platform_version)?;
     // `prove` does not survive decoding (a `DocumentQuery` has no such
     // field), so read it off the wire request before it is consumed.
     let prove = match &request.version {
@@ -791,6 +847,11 @@ pub fn verify_documents_response_with_provider_contract(
     platform_version: &PlatformVersion,
     provider: &dyn ContextProvider,
 ) -> Result<(Option<Documents>, ResponseMetadata, Proof), drive_proof_verifier::Error> {
+    // Same first gate as `verify_documents_response`, run here as well
+    // so an out-of-bounds wire version is rejected before the provider
+    // is asked for anything (the contract lookup below is already
+    // context-provider machinery).
+    check_wire_version_is_served(&request, platform_version)?;
     let contract_id_bytes = match &request.version {
         Some(V0(v0)) => v0.data_contract_id.as_slice(),
         Some(V1(v1)) => v1.data_contract_id.as_slice(),
@@ -1225,23 +1286,28 @@ impl<'a> TryFrom<&'a DocumentQuery> for DriveDocumentQuery<'a> {
         )
         .map_err(Error::Drive)?;
 
-        // `DriveDocumentQuery`'s limit is a `u16`; the wire's is a `u32`.
-        // The server refuses anything above `u16::MAX` outright
-        // (`QuerySyntaxError::InvalidLimit`), so a checked conversion is
-        // what actually mirrors it — an `as` cast would wrap 65537 to a
-        // 1-document query and verify a proof for a query nobody asked
-        // for. `0` keeps its "unset → server default" sentinel meaning.
-        let limit = if request.limit != 0 {
-            Some(u16::try_from(request.limit).map_err(|_| {
-                Error::Config(format!(
-                    "limit {} does not fit a documents query's u16 limit (max {}); \
-                     the server rejects such limits with InvalidLimit",
-                    request.limit,
-                    u16::MAX
-                ))
-            })?)
-        } else {
-            None
+        // Mirror the limit contract of the server's
+        // `DriveDocumentQuery::from_typed_clauses` exactly: `0` (this
+        // struct's "unset" sentinel — V0's `limit: 0`, V1's
+        // `limit: None`) falls back to the server default, and anything
+        // above `DEFAULT_QUERY_LIMIT` (the `config.default_query_limit`
+        // every deployed server runs with) is refused with the server's
+        // own `QuerySyntaxError::InvalidLimit` rather than truncated or
+        // passed through. A `u16::try_from` alone would not do: limits
+        // 101..=65535 fit a `u16` but the server refuses them, so a raw
+        // `DriveDocumentQuery` carrying one would verify a proof no
+        // honest server could have produced.
+        let limit = match request.limit {
+            0 => None,
+            limit if limit > u32::from(DEFAULT_QUERY_LIMIT) => {
+                return Err(Error::Drive(drive::error::Error::Query(
+                    drive::error::query::QuerySyntaxError::InvalidLimit(format!(
+                        "limit {} greater than max limit {}",
+                        limit, DEFAULT_QUERY_LIMIT
+                    )),
+                )));
+            }
+            limit => Some(limit as u16),
         };
 
         let (start_at, start_at_included) = match request.start.as_ref() {

packages/dash-platform-queries/tests/document_query_wire_roundtrip.rs

@@ -294,11 +294,11 @@ fn rejects_contract_mismatch() {
 
 /// A `u32` wire limit above `u16::MAX` must be refused, not wrapped.
 ///
-/// `DriveDocumentQuery`'s limit is a `u16`; the server rejects anything
-/// larger with `InvalidLimit` and so never proves such a query. An `as`
-/// cast would silently turn a request for 65537 documents into a
-/// 1-document query, and a proof for *that* query would then verify —
-/// binding the proof to something the caller never asked for.
+/// The server rejects anything above its query limit cap with
+/// `InvalidLimit` and so never proves such a query. An `as` cast would
+/// silently turn a request for 65537 documents into a 1-document
+/// query, and a proof for *that* query would then verify — binding the
+/// proof to something the caller never asked for.
 #[test]
 fn limit_above_u16_max_is_rejected_not_truncated() {
     let contract = test_contract();
@@ -314,6 +314,39 @@ fn limit_above_u16_max_is_rejected_not_truncated() {
     );
 }
 
+/// The lowering mirrors `DriveDocumentQuery::from_typed_clauses`'
+/// limit contract exactly: `0` = unset → server default (`None`
+/// here), `1..=DEFAULT_QUERY_LIMIT` passes through, and anything
+/// above the cap — including 101..=65535, which fits a `u16` but is
+/// server-invalid — is refused with the server's `InvalidLimit`.
+#[test]
+fn limit_cap_mirrors_the_server() {
+    let contract = test_contract();
+    let query = |limit: u32| {
+        DocumentQuery::new(Arc::clone(&contract), "niceDocument")
+            .expect("document type exists")
+            .with_limit(limit)
+    };
+
+    let unset_query = query(0);
+    let unset = DriveDocumentQuery::try_from(&unset_query).expect("limit 0 is the unset sentinel");
+    assert_eq!(unset.limit, None, "0 must lower to the server default");
+
+    let at_cap_query = query(100);
+    let at_cap =
+        DriveDocumentQuery::try_from(&at_cap_query).expect("the server serves limits up to 100");
+    assert_eq!(at_cap.limit, Some(100));
+
+    for limit in [101u32, 65_535, 65_537, u32::MAX] {
+        let error = DriveDocumentQuery::try_from(&query(limit))
+            .expect_err("a limit the server refuses must not reach a DriveDocumentQuery");
+        assert!(
+            error.to_string().contains("greater than max limit 100"),
+            "unexpected error for limit {limit}: {error}"
+        );
+    }
+}
+
 /// Request-shape checks in [`verify_documents_response`].
 ///
 /// Every field asserted here is dropped by the `DocumentQuery` →
@@ -369,9 +402,11 @@ mod verify_binds_the_whole_request {
     }
 
     /// Encode `query` onto the V1 wire, let `mutate` reshape the request
-    /// the way a hostile transport could, and return the rejection.
-    fn verify_error(
+    /// the way a hostile transport could, and return the rejection
+    /// produced when verifying against `verify_at`.
+    fn verify_error_at_version(
         query: DocumentQuery,
+        verify_at: &PlatformVersion,
         mutate: impl FnOnce(&mut GetDocumentsRequest),
     ) -> drive_proof_verifier::Error {
         let contract = Arc::clone(&query.data_contract);
@@ -385,12 +420,20 @@ mod verify_binds_the_whole_request {
             contract,
             GetDocumentsResponse::default(),
             Network::Testnet,
-            PlatformVersion::latest(),
+            verify_at,
             &NeverCalledProvider,
         )
         .expect_err("the reshaped request must be rejected")
     }
 
+    /// [`verify_error_at_version`] against the latest platform version.
+    fn verify_error(
+        query: DocumentQuery,
+        mutate: impl FnOnce(&mut GetDocumentsRequest),
+    ) -> drive_proof_verifier::Error {
+        verify_error_at_version(query, PlatformVersion::latest(), mutate)
+    }
+
     fn documents_query() -> DocumentQuery {
         DocumentQuery::new(test_contract(), "niceDocument").expect("document type exists")
     }
@@ -467,4 +510,66 @@ mod verify_binds_the_whole_request {
             "unexpected error: {error}"
         );
     }
+
+    /// A V1 wire request verified against a platform version whose
+    /// `document_query` bounds are `0..=0` (protocol version 1) must
+    /// be refused before anything else runs — the server's
+    /// `query_documents` dispatch answers it with
+    /// `UnsupportedQueryVersion` and never proves it. The panicking
+    /// provider pins that the rejection precedes all proof machinery.
+    #[test]
+    fn rejects_wire_version_outside_platform_version_bounds() {
+        let error = verify_error_at_version(documents_query(), v0_platform_version(), |_| {});
+        assert!(
+            error
+                .to_string()
+                .contains("wire version V1 is outside the document_query feature-version bounds"),
+            "unexpected error: {error}"
+        );
+    }
+
+    /// Same wire-version gate on the provider-resolved entry point:
+    /// the rejection must land before the contract lookup, which is
+    /// already context-provider machinery (the provider here panics on
+    /// `get_data_contract`).
+    #[test]
+    fn rejects_wire_version_before_provider_contract_lookup() {
+        use dash_platform_queries::documents::document_query::verify_documents_response_with_provider_contract;
+
+        let request = documents_query()
+            .try_into_request_for_version(v1_platform_version())
+            .expect("query should encode onto the wire");
+
+        let error = verify_documents_response_with_provider_contract(
+            request,
+            GetDocumentsResponse::default(),
+            Network::Testnet,
+            v0_platform_version(),
+            &NeverCalledProvider,
+        )
+        .expect_err("an out-of-bounds wire version must be rejected");
+        assert!(
+            error
+                .to_string()
+                .contains("wire version V1 is outside the document_query feature-version bounds"),
+            "unexpected error: {error}"
+        );
+    }
+
+    /// Limits in `101..=65535` fit the wire's `u32` (and a `u16`) but
+    /// the server refuses them with `InvalidLimit`
+    /// (`DriveDocumentQuery::from_typed_clauses` caps at
+    /// `DEFAULT_QUERY_LIMIT` = 100), so no proved response can belong
+    /// to such a request. The panicking provider pins that the
+    /// rejection precedes all proof machinery.
+    #[test]
+    fn rejects_limit_above_server_cap() {
+        for limit in [101u32, 65_535, u32::MAX] {
+            let error = verify_error(documents_query().with_limit(limit), |_| {});
+            assert!(
+                error.to_string().contains("greater than max limit 100"),
+                "unexpected error for limit {limit}: {error}"
+            );
+        }
+    }
 }