fix(datacell): validate ID ranges in DenseDuplicateTracker deserialization - #2258
Conversation
Merge ProtectionsYour pull request matches the following merge protections and will not be merged until they are valid. 🟢 Require kind labelWonderful, this rule succeeded.
🟢 Require version labelWonderful, this rule succeeded.
🟢 Require linked issue for feature/bug PRsWonderful, this rule succeeded.
|
There was a problem hiding this comment.
Code Review
This pull request adds bounds checking during the deserialization of duplicate trackers (for both standard and legacy formats) to prevent out-of-bounds access, along with corresponding unit tests. The reviewer identified a potential denial-of-service vulnerability where corrupted or malicious files could introduce cycles in duplicate groups, leading to infinite loops or out-of-memory crashes. To mitigate this, the reviewer suggested adding checks to ensure that deserialized IDs are not already part of another duplicate group.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds defensive bounds validation during DenseDuplicateTracker deserialization to prevent out-of-range IDs from corrupting memory when reading potentially malformed/corrupted index files.
Changes:
- Add bounds checks for
head_id/dup_idinDeserializeandid/duplicate_idinDeserializeFromLegacyFormat. - Throw
VsagException(ErrorType::INVALID_BINARY, ...)when encountering out-of-range IDs. - Add unit tests covering out-of-range IDs for both current and legacy formats.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/datacell/dense_duplicate_tracker.cpp | Adds range checks and throws on invalid IDs during deserialization paths. |
| src/datacell/dense_duplicate_tracker_test.cpp | Adds negative tests asserting invalid IDs are rejected via exceptions. |
54d1cb6 to
91dbce6
Compare
91dbce6 to
301bbea
Compare
…ation Add bounds checks for head_id, dup_id, and all IDs read from the stream in both Deserialize and DeserializeFromLegacyFormat. Without validation, a corrupted or malicious index file could provide IDs exceeding the vector size, causing heap buffer overflow writes via duplicate_ids_[id]. Throw VsagException with INVALID_BINARY when any ID is out of range. Signed-off-by: tianlan.lht <tianlan.lht@antgroup.com> Signed-off-by: LHT129 <tianlan.lht@antgroup.com>
301bbea to
884e4c5
Compare
…ation (antgroup#2258) fix(hgraph): add bounds checks on IDs during label table deserialization Without validation, a corrupted or malicious index file could provide head_id, dup_id, or stream-read IDs exceeding the vector size, causing heap buffer overflow writes via duplicate_ids_[id]. Add bounds checks for all such IDs in both Deserialize and DeserializeFromLegacyFormat; throw VsagException(INVALID_BINARY) on any out-of-range value. Assisted-by: OpenCode:claude-opus-4.7 Signed-off-by: LHT129 <tianlan.lht@antgroup.com> Signed-off-by: Sia Sheerland <x1075956441x@163.com> Signed-off-by: Sia Sheerland <x1075956441x@163.com>
Summary
Fixes #2257
Add comprehensive validation for all IDs read from the stream in
DenseDuplicateTracker::DeserializeandDeserializeFromLegacyFormat. Without validation, a corrupted or malicious index file could cause heap buffer overflow writes or infinite loops.Changes
Bounds checks (heap overflow prevention)
head_idand eachdup_idagainstsizeinDeserializeidand eachduplicate_idagainsttotal_sizeinDeserializeFromLegacyFormatOverlap checks (infinite loop prevention)
head_id/idthat is already part of another group (duplicate_ids_[x] != x)dup_id/duplicate_idthat is already assigned to another groupError message improvements
fmt::formatto include the offending ID value and the valid boundsVsagException(ErrorType::INVALID_BINARY, ...)for all validation failuresTest Plan
DeserializeandDeserializeFromLegacyFormatpaths tested