fix(cypher): bound OPTIONAL fallback in expand_pattern_rels to a single ceiling#1177
Conversation
7891465 to
a3adfbc
Compare
|
Thanks for isolating a genuine start-bound OPTIONAL expansion overflow. The current fix needs revision before further review:
This feedback is for reviewed head |
a3adfbc to
fe548db
Compare
|
Thanks revised to address all four points:
|
…le ceiling Signed-off-by: SEPURI-SAI-KRISHNA <saik20533@gmail.com>
fe548db to
8763aa9
Compare
What does this PR do?
Fixes a heap buffer overflow (CWE-787) in
expand_pattern_rels(
src/cypher/cypher.c) when expanding anOPTIONAL MATCHrelationship pattern.The query text is agent-controlled via the MCP
querytool.The bug
The per-hop output buffer was sized
bind_cap * 10 + 1:The expansion helpers (
expand_fixed_length/expand_var_length, viaprocess_edges) correctly stop atmax_new = bind_cap * 10. But the OPTIONALfallback that keeps a row with the target left unbound was not bounded:
The
+ 1left room for a single fallback row after a saturated expansion.When one source binding saturates the expansion to
max_newand two or morelater sources take the OPTIONAL (no-match) path, the second fallback write runs
past the allocation.
Reachable whenever
bind_captracks the scanned node count — i.e. the number ofmatched start nodes exceeds the result limit — with e.g.:
That regime is hit on a repo with more start nodes than the supplied
max_rows(or, at the default limit, more than the 100000 result ceiling). A dense hub
(out-degree ≥
bind_cap * 10) saturates the buffer; the leaf nodes that followon the OPTIONAL path then overflow it.
The fix
Hold the OPTIONAL fallback to the same single global ceiling (
max_new = bind_cap * 10) the expansion helpers already respect, instead of growing theallocation. The buffer is sized for the ceiling, and every writer — expansion
and fallback alike — stops there:
Behavior at the ceiling is now defined: the hop truncates at
max_new(bounded success, no overflow, no error), matching how the sibling
expand_from_bound_terminalalready bounds its own fallback.Reproduced first (ASan, before the fix):
After the fix the full
cyphersuite passes with no sanitizer errors.Tests
Two regression tests:
cypher_exec_optional_rel_ceiling_truncates_no_overflow— a hub thatsaturates the expansion followed by leaf functions on the OPTIONAL path;
asserts the query still succeeds with a bounded result (defined truncation, no
overflow).
cypher_exec_optional_rel_leaf_fallback_survives— a non-saturating graph;asserts a specific leaf's OPTIONAL row survives with its target unbound and
that a real expanded hub row is also present (a
row_count > 0check would betoo weak — it can pass on hub rows alone).
Checklist
git commit -s) — required, CI rejectsunsigned commits (DCO, see CONTRIBUTING.md)
make -f Makefile.cbm test)