Content-Length: 349078 | pFad | http://github.com/ClickHouse/ClickHouse/pull/112331

16 Fix out-of-bounds read in SerializationReplicated on the native protocol by Avogar · Pull Request #112331 · ClickHouse/ClickHouse · GitHub
Skip to content

Fix out-of-bounds read in SerializationReplicated on the native protocol - #112331

Open
Avogar wants to merge 2 commits into
masterfrom
fix-replicated-serialization-oob-index
Open

Fix out-of-bounds read in SerializationReplicated on the native protocol#112331
Avogar wants to merge 2 commits into
masterfrom
fix-replicated-serialization-oob-index

Conversation

@Avogar

@Avogar Avogar commented Jul 28, 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 out-of-bounds read in SerializationReplicated on the native protocol on corrupted indexes data.

`SerializationReplicated::deserializeBinaryBulkWithMultipleStreams` read a
client-supplied replicated-index buffer from the native TCP protocol and
validated only the index row count, not the index values. Every
`ColumnReplicated` accessor then dereferences `nested_column[index]` without
a bound, so a crafted `REPLICATED`-serialized column whose indexes point past
the nested ("elements") column produces an out-of-bounds read when the column
is materialized.

Validate every deserialized index against the number of nested elements
(throwing `INCORRECT_DATA`) before attaching the indexes, mirroring the
per-discriminator check already done in `SerializationVariant`.

Reachable only over the native TCP protocol (negotiated revision >=
`DBMS_MIN_REVISION_WITH_REPLICATED_SERIALIZATION`); the format layer builds
`NativeReader` with `server_revision = 0`, so `REPLICATED` is never read there.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@clickhouse-gh

clickhouse-gh Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Workflow [PR], commit [b55379a]

@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 28, 2026
Comment thread src/DataTypes/Serializations/SerializationReplicated.cpp
@clickhouse-gh

clickhouse-gh Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

LLVM Coverage Report

Metric Baseline Current Δ
Lines 86.40% 86.30% -0.10%
Functions 92.10% 92.10% +0.00%
Branches 78.50% 78.40% -0.10%

Changed lines: Changed C/C++ lines covered: 20/23 (86.96%) · Uncovered code

Full report · Diff report

`checkDeserializedIndexes` only proved that replicated indexes are less
than the advertised `num_elements`; it never proved that
`nested->deserializeBinaryBulkWithMultipleStreams` actually produced
`num_elements` rows. Primitive bulk readers such as
`SerializationNumber::deserializeBinaryBulk` short-read on EOF instead of
throwing, and `NativeReader::readData` only checks the top-level
`column->size()`, which for `ColumnReplicated` is the index count, not the
nested column size. So a client could send in-range indexes, advertise
`num_elements = N`, truncate the elements payload, and still hand an
out-of-bounds-capable `ColumnReplicated` to the executor.

Verify `column_replicated.getNestedColumn()->size() == num_elements` after
the nested deserialization and throw `INCORRECT_DATA` on mismatch,
mirroring the post-read consistency checks in `SerializationArray` and
`SerializationVariant`. Extend the regression test with a second scenario
covering in-range indexes plus a truncated elements stream.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-critical-bugfix pr-must-backport Pull request should be backported intentionally. Use this label with great care!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant









ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: http://github.com/ClickHouse/ClickHouse/pull/112331

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy