Skip to content

Fix OOB segfault in groupArrayLastMerge#109485

Merged
mstetsyuk merged 1 commit into
masterfrom
fix/grouparraylast-merge-oob
Jul 8, 2026
Merged

Fix OOB segfault in groupArrayLastMerge#109485
mstetsyuk merged 1 commit into
masterfrom
fix/grouparraylast-merge-oob

Conversation

@mstetsyuk

@mstetsyuk mstetsyuk commented Jul 6, 2026

Copy link
Copy Markdown
Member

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):

Fix a segfault in groupArrayLastMerge. Deserialization of aggregated function state is now validated. So a broken state does not lead to OOB.

Version info

  • Merged into: 26.7.1.670 (included in 26.7 and later)
  • Backported to: 26.6.2.53, 26.5.6.64, 26.4.5.152, 26.3.17.66, 25.8.29.28

@clickhouse-gh

clickhouse-gh Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Workflow [PR], commit [c67534b]

Summary:


AI Review

Summary

This PR hardens groupArrayLast and groupArraySample state deserialization by rejecting states whose stored element count is inconsistent with total_values and max_elems, which closes the malformed-state path that could previously read past the stored array during merge. I did not find a remaining correctness issue in the code change itself; the only review note is a changelog-entry wording nit, and runtime confirmation is still pending because the direct Praktika report is empty.

PR Metadata
  • Changelog category: Critical Bug Fix matches the change.
  • Changelog entry: required, but the current text is too implementation-focused (OOB, "broken state") and does not follow the present-tense, user-facing style from the changelog guidelines.
  • Replacement: Fixes a server exception when malformed \groupArrayLast` or `groupArraySample` aggregate states are deserialized or merged.`
Missing context / blind spots
  • ⚠️ The direct Praktika report for PR #109485 was still empty at review time (Total: 0), and this checkout has no existing build directory, so I could not independently run 02520_group_array_last to confirm the new regression coverage.
Final Verdict

✅ No blocking issues found in the patch. Merge is reasonable once CI executes and the changelog entry is rewritten in user-facing terms.

@clickhouse-gh clickhouse-gh Bot added pr-critical-bugfix pr-must-backport Pull request should be backported intentionally. Use this label with great care! labels Jul 6, 2026
@yakov-olkhovskiy yakov-olkhovskiy self-assigned this Jul 6, 2026
@clickhouse-gh

clickhouse-gh Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

LLVM Coverage Report

Metric Baseline Current Δ
Lines 85.60% 85.60% +0.00%
Functions 92.70% 92.70% +0.00%
Branches 77.80% 77.80% +0.00%

Changed lines: Changed C/C++ lines covered: 17/17 (100.00%) · Uncovered code

Full report · Diff report

@mstetsyuk
mstetsyuk added this pull request to the merge queue Jul 8, 2026
Merged via the queue into master with commit 9e76891 Jul 8, 2026
175 checks passed
@mstetsyuk
mstetsyuk deleted the fix/grouparraylast-merge-oob branch July 8, 2026 14:56
@robot-ch-test-poll2 robot-ch-test-poll2 added the pr-synced-to-cloud The PR is synced to the cloud repo label Jul 8, 2026
@robot-clickhouse-ci-2 robot-clickhouse-ci-2 added the pr-must-backport-synced The `*-must-backport` labels are synced into the cloud Sync PR label Jul 8, 2026
@robot-clickhouse-ci-2 robot-clickhouse-ci-2 added the pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore label Jul 8, 2026
clickhouse-gh Bot added a commit that referenced this pull request Jul 8, 2026
Backport #109485 to 26.6: Fix OOB segfault in `groupArrayLastMerge`
@tavplubix tavplubix added v25.12-must-backport v26.2-must-backport v26.4-must-backport v26.6-must-backport and removed pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore pr-must-backport-synced The `*-must-backport` labels are synced into the cloud Sync PR labels Jul 22, 2026
@robot-ch-test-poll3 robot-ch-test-poll3 added pr-must-backport-synced The `*-must-backport` labels are synced into the cloud Sync PR pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore labels Jul 22, 2026
mstetsyuk added a commit that referenced this pull request Jul 22, 2026
Backport #109485 to 25.8: Fix OOB segfault in `groupArrayLastMerge`
mstetsyuk added a commit that referenced this pull request Jul 22, 2026
Backport #109485 to 26.3: Fix OOB segfault in `groupArrayLastMerge`
mstetsyuk added a commit that referenced this pull request Jul 22, 2026
Backport #109485 to 26.4: Fix OOB segfault in `groupArrayLastMerge`
mstetsyuk added a commit that referenced this pull request Jul 22, 2026
Backport #109485 to 26.5: Fix OOB segfault in `groupArrayLastMerge`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore pr-critical-bugfix pr-must-backport Pull request should be backported intentionally. Use this label with great care! pr-must-backport-synced The `*-must-backport` labels are synced into the cloud Sync PR pr-synced-to-cloud The PR is synced to the cloud repo v25.12-must-backport v26.2-must-backport v26.4-must-backport v26.6-must-backport

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants