fix(cypher): bounds-check and error-propagate cross_join_nodes allocation

[SEPURI-SAI-KRISHNA]

What does this PR do?

Fixes a heap buffer overflow (CWE-787) in cross_join_nodes
(src/cypher/cypher.c), reached when an additional MATCH / OPTIONAL MATCH
node pattern is cross-joined into the current bindings. The query text is
agent-controlled via the MCP query tool.

The reachable bug

When an OPTIONAL MATCH's label matches zero nodes, cross_join_nodes is
called with extra_count == 0. The allocation was (bind_count * 0 + 1) =
one binding slot, but the OPTIONAL fallback then writes one binding per
existing row (bind_count of them). Any query whose first pattern binds ≥ 2
nodes and whose OPTIONAL pattern matches zero nodes overflows the heap:

MATCH (a:Function) OPTIONAL MATCH (b:NoSuchLabel) RETURN a.name

Reproduced first (ASan, before the fix — 4 nodes, no large graph needed):

==ERROR: AddressSanitizer: heap-buffer-overflow ... WRITE of size 1624
    #0 cross_join_nodes src/cypher/cypher.c   (OPTIONAL fallback write)
       allocated at cross_join_nodes src/cypher/cypher.c  (1-slot malloc)
    #5 test_cypher_exec_optional_empty_label_no_overflow
SUMMARY: AddressSanitizer: heap-buffer-overflow ... in cross_join_nodes

The fix sizes the OPTIONAL-empty case as bind_count entries so the buffer
holds every fallback row. After the fix the query returns one row per bound node
(b left unbound — correct OPTIONAL dead-code semantics).

Allocation-arithmetic hardening (per review)

The same allocation math is now fully bounds-checked, via a small helper so the
boundary is unit-testable:

• Checked byte size. alloc_n * sizeof(binding_t) is rejected if it would
  overflow size_t.
• INT_MAX guard. The row count is computed in size_t; if it would not fit
  the int binding counter (*bind_count = (int)new_count) the join is refused
  rather than silently truncating the count. A plain-int
bind_count * extra_count product would wrap past INT_MAX on a large graph
  (e.g. an unbounded Cartesian MATCH (a:X) MATCH (b:Y)).
• Error propagation. On a refused or failed allocation cross_join_nodes
  now returns an error that expand_additional_patterns → execute_single →
  cbm_cypher_execute surface as a query error (out->error), instead of
  leaving the original bindings in place and silently skipping the MATCH — that
  would return a wrong (short) result.

int cbm_cypher_cross_join_alloc(int bind_count, int extra_count, bool opt, size_t *out_n) {
    size_t per_binding = extra_count > 0 ? (size_t)extra_count : (opt ? (size_t)SKIP_ONE : 0U);
    size_t rows = (size_t)bind_count * per_binding;
    if (rows > (size_t)INT_MAX) {
        return CBM_NOT_FOUND;
    }
    size_t n = rows + SKIP_ONE;
    if (n > SIZE_MAX / sizeof(binding_t)) {
        return CBM_NOT_FOUND;
    }
    *out_n = n;
    return 0;
}

Tests

• cypher_exec_optional_empty_label_no_overflow — the reachable zero-label
  overflow (RED under ASan before the fix; returns 4 rows after).
• cypher_cross_join_alloc_rejects_overflow — an arithmetic-boundary test that
  drives the helper with 46341 * 46341 > INT_MAX and asserts it is rejected
  (RED-confirmed: with the guards disabled the helper returns success and the
  test fails), plus normal counts (4×3 → 13, OPTIONAL 4×0 → 5, 4×0 → 1).

The full cypher suite passes with no sanitizer errors (161 tests).

Checklist

• Every commit is signed off (git commit -s) — required, CI rejects
  unsigned commits (DCO, see CONTRIBUTING.md)
• Tests pass locally (make -f Makefile.cbm test)
• New behavior is covered by a test (reproduce-first for bug fixes)

[DeusData]

Thanks for the focused reproduction. The zero-result OPTIONAL MATCH overflow is real, but exact-head review found the broader large-product hardening incomplete:

1. alloc_n * sizeof(binding_t) still needs a checked byte-size multiplication before allocation.
2. The size_t new_count to int assignment needs an INT_MAX guard.
3. Allocation failure must propagate a query error; retaining the original bindings silently skips the MATCH and can return a wrong result.
4. Add a RED arithmetic-boundary test for the claimed large-product path, not only the reachable zero-match case.

Please keep the narrow fix, complete those checks and error semantics, and either link a matching issue or remove the broader #627 association. This feedback is for reviewed head 12cdf2824c68349f3995560e1f1b99cf45ad8ddf.

[SEPURI-SAI-KRISHNA]

@DeusData Thanks, all four addressed:

1. Checked byte size. alloc_n * sizeof(binding_t) is now rejected if it would overflow size_t.
2. INT_MAX guard. The row count is computed in size_t and refused if it would not fit the int binding counter, so *bind_count = (int)new_count is provably safe.
3. Error propagation. A refused/failed allocation now returns an error that expand_additional_patterns → execute_single → cbm_cypher_execute surface as out->error, instead of silently retaining the old bindings and skipping the MATCH.
4. RED arithmetic-boundary test. Both (1) and (2) live in a small helper cbm_cypher_cross_join_alloc, unit-tested by cypher_cross_join_alloc_rejects_overflow at 46341 * 46341 > INT_MAX. Confirmed RED→GREEN: with the guards disabled the helper returns success and the test fails; with them in place the full cypher suite is clean (161 passed, full suite 6225 passed).

Dropped the broader #627 association from the description; this PR is scoped to
the reachable zero-label overflow plus the allocation-arithmetic hardening.

[DeusData]

Thanks for addressing all four requested points and for tightening the #627 claim. I see current head b07dbe67ad16d62d16a003d4d46bb34cd13cf627. The checked-size arithmetic, INT_MAX guard, propagated allocation error, and focused boundary helper are the right response shape. No further contributor action is requested now; this is back in exact-head maintainer review.