Skip to content

Commit cc1828d

Browse files
fix(verify): reject empty-path aggregate-sum/count queries at validation
Codex follow-up + CodeRabbit: the previous fix added a terminal-type gate in `enforce_lower_chain`, but `verify_v0_layer` and `verify_v1_layer` short-circuit to the leaf verifier when `depth == path_keys.len()`. With an empty path (`path == []`) that's true at depth 0, so the type gate is never invoked. In practice the empty-path case is already protected by hash divergence: the GroveDB root merk is always a `NormalTree` (built with `Element::empty_tree()` by API), so its root_hash uses `node_hash`. An attacker's forged proof of `HashWithSum` / `HashWithCount` ops would reconstruct via `node_hash_with_sum` / `node_hash_with_count` — distinct hash functions, no collision. So the caller's root-hash compare catches the forgery cryptographically. But the defense-in-depth principle says: don't rely on the cryptographic divergence implicitly. Reject up-front, before any proof handling. PathQuery::validate_aggregate_{sum,count}_on_range now check `self.path.is_empty()` and return a clear InvalidQuery error naming why (root is always NormalTree, no valid Provable* target at root). The check fires at the entry of `verify_aggregate_{sum,count}_query` (which call `validate_*` first thing) and at `prove_query` (the generator also validates the path query before dispatch). TESTS - `empty_path_aggregate_sum_rejected_at_validation` - `empty_path_aggregate_count_rejected_at_validation` Both pin the rejection at both the PathQuery validator and the verify entrypoint. 2964 workspace tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent da53ef0 commit cc1828d

3 files changed

Lines changed: 109 additions & 2 deletions

File tree

grovedb/src/query/mod.rs

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -222,16 +222,48 @@ impl PathQuery {
222222
/// `AggregateCountOnRange` query. On success, returns a reference to the
223223
/// inner range item.
224224
///
225+
/// Rejects empty paths up-front. The GroveDB root merk is always a
226+
/// `NormalTree` by API construction (and never a `ProvableCountTree`),
227+
/// so a root-level aggregate-count query has no valid target —
228+
/// `verify_v0_layer` and `verify_v1_layer` would otherwise hit the
229+
/// `depth == path_keys.len()` short-circuit at depth 0, going
230+
/// straight to the merk-level count verifier without ever invoking
231+
/// the terminal-type gate in `enforce_lower_chain`. Although the
232+
/// merk-level hash-divergence between `node_hash` and
233+
/// `node_hash_with_count` makes a numeric forgery infeasible, an
234+
/// up-front rejection gives a clear error and removes the gate
235+
/// dependency on cryptographic hash analysis.
236+
///
225237
/// Forwards to [`SizedQuery::validate_aggregate_count_on_range`].
226238
pub fn validate_aggregate_count_on_range(&self) -> Result<&QueryItem, Error> {
239+
if self.path.is_empty() {
240+
return Err(Error::InvalidQuery(
241+
"AggregateCountOnRange queries may not target the root merk: \
242+
the GroveDB root is always a NormalTree, never a \
243+
ProvableCountTree / ProvableCountSumTree, so a count \
244+
aggregate at the root layer has no valid target",
245+
));
246+
}
227247
self.query.validate_aggregate_count_on_range()
228248
}
229249

230250
/// Validates that this `PathQuery` is a well-formed
231251
/// `AggregateSumOnRange` query. On success, returns a reference to the
232-
/// inner range item. Forwards to
233-
/// [`SizedQuery::validate_aggregate_sum_on_range`].
252+
/// inner range item.
253+
///
254+
/// Rejects empty paths up-front for the same reason as
255+
/// [`Self::validate_aggregate_count_on_range`] — the GroveDB root
256+
/// merk is always a `NormalTree`, never a `ProvableSumTree`. Forwards
257+
/// to [`SizedQuery::validate_aggregate_sum_on_range`].
234258
pub fn validate_aggregate_sum_on_range(&self) -> Result<&QueryItem, Error> {
259+
if self.path.is_empty() {
260+
return Err(Error::InvalidQuery(
261+
"AggregateSumOnRange queries may not target the root merk: \
262+
the GroveDB root is always a NormalTree, never a \
263+
ProvableSumTree, so a sum aggregate at the root layer has \
264+
no valid target",
265+
));
266+
}
235267
self.query.validate_aggregate_sum_on_range()
236268
}
237269

grovedb/src/tests/aggregate_count_query_tests.rs

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1231,6 +1231,42 @@ mod tests {
12311231
}
12321232
}
12331233

1234+
/// Security regression: empty-path aggregate-count queries are
1235+
/// rejected at validation time, before any proof handling.
1236+
///
1237+
/// `verify_aggregate_count_query` calls
1238+
/// `path_query.validate_aggregate_count_on_range()` at its entry. If
1239+
/// the path is empty, validation must fail — otherwise both
1240+
/// `verify_v0_layer` and `verify_v1_layer` would hit the
1241+
/// `depth == path_keys.len()` short-circuit at depth 0 and go
1242+
/// straight to the merk-level leaf verifier, never invoking the
1243+
/// terminal-type gate in `enforce_lower_chain`. The GroveDB root
1244+
/// merk is always a `NormalTree` by API construction, so a root
1245+
/// aggregate-count query has no valid target.
1246+
#[test]
1247+
fn empty_path_aggregate_count_rejected_at_validation() {
1248+
let v = GroveVersion::latest();
1249+
let pq = PathQuery::new_aggregate_count_on_range(
1250+
Vec::new(),
1251+
QueryItem::RangeFrom(b"a".to_vec()..),
1252+
);
1253+
let err = pq
1254+
.validate_aggregate_count_on_range()
1255+
.expect_err("empty path must be rejected at validation");
1256+
let msg = format!("{err}");
1257+
assert!(
1258+
msg.contains("root")
1259+
&& (msg.contains("ProvableCountTree") || msg.contains("ProvableCountSumTree")),
1260+
"expected message naming root + ProvableCountTree, got: {msg}"
1261+
);
1262+
1263+
let result = GroveDb::verify_aggregate_count_query(&[0u8; 4], &pq, v);
1264+
assert!(
1265+
result.is_err(),
1266+
"verify_aggregate_count_query must reject empty-path queries"
1267+
);
1268+
}
1269+
12341270
/// Security regression: empty-leaf type-confusion forgery
12351271
/// (parallel of `empty_leaf_type_confusion_forgery_rejected` on the
12361272
/// sum side).

grovedb/src/tests/aggregate_sum_query_tests.rs

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1050,6 +1050,45 @@ mod tests {
10501050
}
10511051
}
10521052

1053+
/// Security regression: empty-path aggregate-sum queries are
1054+
/// rejected at validation time, before any proof handling.
1055+
///
1056+
/// `verify_aggregate_sum_query` calls
1057+
/// `path_query.validate_aggregate_sum_on_range()` at its entry. If
1058+
/// the path is empty, validation must fail — otherwise both
1059+
/// `verify_v0_layer` and `verify_v1_layer` would hit the
1060+
/// `depth == path_keys.len()` short-circuit at depth 0 and go
1061+
/// straight to the merk-level leaf verifier, never invoking the
1062+
/// terminal-type gate in `enforce_lower_chain`. The GroveDB root
1063+
/// merk is always a `NormalTree` by API construction, so a root
1064+
/// aggregate-sum query has no valid target.
1065+
#[test]
1066+
fn empty_path_aggregate_sum_rejected_at_validation() {
1067+
let v = GroveVersion::latest();
1068+
let pq = PathQuery::new_aggregate_sum_on_range(
1069+
Vec::new(), // empty path → must be rejected
1070+
QueryItem::RangeFrom(b"a".to_vec()..),
1071+
);
1072+
let err = pq
1073+
.validate_aggregate_sum_on_range()
1074+
.expect_err("empty path must be rejected at validation");
1075+
let msg = format!("{err}");
1076+
assert!(
1077+
msg.contains("root") && msg.contains("ProvableSumTree"),
1078+
"expected message naming root + ProvableSumTree, got: {msg}"
1079+
);
1080+
1081+
// Also confirm the verifier surface rejects with the same error
1082+
// (the validator is called first inside verify_aggregate_sum_query).
1083+
// We don't need a real proof — any bytes go in; validation runs
1084+
// before proof decode.
1085+
let result = GroveDb::verify_aggregate_sum_query(&[0u8; 4], &pq, v);
1086+
assert!(
1087+
result.is_err(),
1088+
"verify_aggregate_sum_query must reject empty-path queries"
1089+
);
1090+
}
1091+
10531092
/// Same forgery shape, but the honest leaf is an empty
10541093
/// `ProvableCountTree` (the wrong PROVABLE tree type for a sum
10551094
/// query). Confirms the terminal-type gate enforces the precise

0 commit comments

Comments
 (0)