Skip to content

Conversation

@777genius
Copy link

@777genius 777genius commented Nov 9, 2025

Summary

This PR adds missing error checking for binary.Uvarint calls in history_index_block.go to prevent potential infinite loops and crashes when processing corrupted index block data.

Problem

Missing validation in four locations can lead to serious issues:

  1. Line 179 (readGreaterThan): Ignores n return value when decoding restart item
  2. Line 203 (readGreaterThan loop): No n <= 0 check - CRITICAL infinite loop risk
  3. Line 223 (readGreaterThan): Ignores n return value via binary search
  4. Line 303 (scanSection loop): No n <= 0 check - CRITICAL infinite loop risk

When binary.Uvarint returns n <= 0, it indicates:

  • n == 0: Buffer too small (incomplete varint)
  • n < 0: Value overflow (> 64 bits)

Without checks, loops can:

  • Run infinitely when n == 0 (pos += 0)
  • Access invalid memory when n < 0 (negative offset)
  • Silently use garbage values

Solution

Added validation following the existing pattern from lines 169-172:

item, n := binary.Uvarint(data)
if n <= 0 {
    return 0, fmt.Errorf("failed to decode...")
}

For scanSection, used silent break to avoid breaking the internal API.

Testing

Added comprehensive tests:

  • TestBlockReaderCorruptedVarint: Tests all three readGreaterThan code paths
  • TestBlockWriterCorruptedVarint: Tests scanSection graceful handling
  • All existing tests pass (17s runtime)

Context

This follows recent similar fixes in the module:

  • d2a5dba: "triedb/pathdb: fix 32-bit integer overflow in history trienode decoder"
  • 11c0fb9: "triedb/pathdb: fix index out of range panic in decodeSingle"

…tions

This commit adds missing error checking for binary.Uvarint calls in
history_index_block.go to prevent potential infinite loops and crashes
when processing corrupted index block data.

Missing validation could lead to:
- Infinite loops when n == 0 (incomplete varint at end of buffer)
- Negative position values when n < 0 (varint overflow)
- Silent data corruption being ignored

Changes:
- Add n <= 0 validation in readGreaterThan() at lines 179, 203, 223
- Add n <= 0 validation in scanSection() at line 303 with silent break
- Add comprehensive tests for corrupted varint handling
- Follow existing validation pattern from lines 169-172

This follows the same approach as recent fixes in this module:
- Commit d2a5dba: "triedb/pathdb: fix 32-bit integer overflow"
- Commit 11c0fb9: "triedb/pathdb: fix index out of range panic"
@777genius 777genius force-pushed the fix/pathdb-varint-validation branch from 0cf88ab to f5d5a69 Compare November 9, 2025 18:54
@rjl493456442
Copy link
Member

We're not interested in reviewing PRs generated by AI.

While some of them might be valid, it's unfair that AI-generated PRs can be created endlessly, while maintainers have to spend significant effort ensuring all the changes are correct and safe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants