validate event type bounds in windowFunnel state deserialization

[uwezkhan]

AddressSanitizer, getEventLevelNonStrictOnce on a crafted state whose single event byte is 255:

READ of size 1 ... AggregateFunctionWindowFunnel.cpp:347
  events_timestamp[event_idx - 1]   // event_idx = 254, buffer holds events_size elements

windowFunnel reads each event's condition index from the serialized state without a range check, then getEventLevelNonStrictOnce / getEventLevelStrictOnce use event - 1 to index events_timestamp / event_sequences (both sized events_size, the number of conditions, at most 32) and a std::array<UInt64, 32>. An event byte of 255 indexes about 250 elements past the buffer (out-of-bounds read, then a write at events_timestamp[event - 1]); an event byte of 0 with strict_order off indexes before it. The legitimate add path only produces events in [0, events_size], so deserialize is the only place such values enter. Reachable by any user:

SELECT finalizeAggregation(CAST(unhex('01010000000000000000000000FF') AS AggregateFunction(windowFunnel(1), UInt32, UInt8, UInt8)));

Before: out-of-range event types from a deserialized state reach the indexing loops.
After: deserialize rejects any event outside [1, events_size] (0 only with strict_order, the no-event sentinel) with INCORRECT_DATA; both the default and strict_once state layouts are covered.

The check is one linear pass over the deserialized events, negligible next to the deserialization itself.

Changelog category (leave one):

• Critical Bug Fix (crash, data loss, RBAC)

Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):

Fixed a heap buffer overflow (server crash) in windowFunnel when finalizing a crafted aggregate-function state with an out-of-range event type, reachable by any user with a single SELECT.

Version info

• Backported to: 26.5.4.9, 26.4.5.81, 26.3.16.4, 25.8.25.33

[clickhouse-gh]

Workflow [PR], commit [ea2e95b]

Summary: ✅

• Performance Comparison: Performance dashboard

---

AI Review

Summary

This PR hardens windowFunnel aggregate-state deserialization by rejecting event types outside the function's condition range while preserving the strict_order zero-event sentinel compatibility path. The current code and focused stateless test cover the default layout, strict_once, and the strict_order compatibility cases; I found no unresolved blocker or major issue.

Final Verdict

Status: ✅ Approve

[alexey-milovidov]

LGTM

[uwezkhan]

Also fixed the arm_tidy build failure: event_type was flagged by cppcoreguidelines-init-variables since it was declared before the if constexpr branches set it, so it's now initialized at declaration.

[clickhouse-gh]

LLVM Coverage Report

Metric	Baseline	Current	Δ
Lines	85.30%	85.30%	+0.00%
Functions	92.50%	92.60%	+0.10%
Branches	77.50%	77.60%	+0.10%

Changed lines: Changed C/C++ lines covered by tests: 12/12 (100.00%) | Lost baseline coverage (was covered on master, now uncovered in this PR): 1 line(s) · Uncovered code

Full report · Diff report