Skip to content

Commit f5d5a69

Browse files
committed
triedb/pathdb: add validation for binary.Uvarint in index block operations
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"
1 parent ebc7dc9 commit f5d5a69

File tree

2 files changed

+144
-2
lines changed

2 files changed

+144
-2
lines changed

triedb/pathdb/history_index_block.go

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,10 @@ func (br *blockReader) readGreaterThan(id uint64) (uint64, error) {
176176
return 0, err
177177
}
178178
if index == 0 {
179-
item, _ := binary.Uvarint(br.data[br.restarts[0]:])
179+
item, n := binary.Uvarint(br.data[br.restarts[0]:])
180+
if n <= 0 {
181+
return 0, fmt.Errorf("failed to decode item at restart %d", br.restarts[0])
182+
}
180183
return item, nil
181184
}
182185
var (
@@ -198,6 +201,9 @@ func (br *blockReader) readGreaterThan(id uint64) (uint64, error) {
198201
pos := start
199202
for pos < limit {
200203
x, n := binary.Uvarint(br.data[pos:])
204+
if n <= 0 {
205+
return 0, fmt.Errorf("failed to decode varint at position %d", pos)
206+
}
201207
if pos == start {
202208
result = x
203209
} else {
@@ -214,7 +220,10 @@ func (br *blockReader) readGreaterThan(id uint64) (uint64, error) {
214220
}
215221
// The element which is the first one greater than the specified id
216222
// is exactly the one located at the restart point.
217-
item, _ := binary.Uvarint(br.data[br.restarts[index]:])
223+
item, n := binary.Uvarint(br.data[br.restarts[index]:])
224+
if n <= 0 {
225+
return 0, fmt.Errorf("failed to decode item at restart %d", br.restarts[index])
226+
}
218227
return item, nil
219228
}
220229

@@ -291,6 +300,10 @@ func (b *blockWriter) scanSection(section int, fn func(uint64, int) bool) {
291300
}
292301
for pos < limit {
293302
x, n := binary.Uvarint(b.data[pos:])
303+
if n <= 0 {
304+
// Skip corrupted entry
305+
break
306+
}
294307
if pos == start {
295308
value = x
296309
} else {

triedb/pathdb/history_index_block_test.go

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,3 +251,132 @@ func BenchmarkBlockWriterAppend(b *testing.B) {
251251
}
252252
}
253253
}
254+
255+
// TestBlockReaderCorruptedVarint tests that readGreaterThan properly handles
256+
// corrupted varint encoding and returns errors instead of hanging or panicking.
257+
func TestBlockReaderCorruptedVarint(t *testing.T) {
258+
// Create a valid block with multiple restart sections
259+
elements := []uint64{1, 5, 10, 11, 20, 100, 200, 300, 400, 500}
260+
bw, _ := newBlockWriter(nil, newIndexBlockDesc(0))
261+
for i := 0; i < len(elements); i++ {
262+
bw.append(elements[i])
263+
}
264+
validBlob := bw.finish()
265+
266+
t.Run("corrupted varint at first restart", func(t *testing.T) {
267+
// Corrupt the first byte of the first restart to create invalid varint
268+
blob := slices.Clone(validBlob)
269+
restarts, data, _ := parseIndexBlock(blob)
270+
if len(restarts) > 0 {
271+
// Set the first byte to 0xFF repeatedly to create invalid varint
272+
// (varint encoding where all bytes have high bit set is invalid)
273+
for i := 0; i < 10 && int(restarts[0])+i < len(data); i++ {
274+
data[int(restarts[0])+i] = 0xFF
275+
}
276+
}
277+
278+
br, err := newBlockReader(blob)
279+
if err != nil {
280+
// parseIndexBlock might catch it first, which is also acceptable
281+
return
282+
}
283+
// Try to read, should get error instead of hanging
284+
_, err = br.readGreaterThan(0)
285+
if err == nil {
286+
t.Fatal("Expected error when reading corrupted varint at first restart, got nil")
287+
}
288+
})
289+
290+
t.Run("truncated varint in loop", func(t *testing.T) {
291+
// Create a block with two values: 5 and then truncated varint
292+
// Data: [0x05] (value 5) [0x80] (incomplete varint - truncated delta)
293+
// Restart: [0x00, 0x00] (points to offset 0)
294+
// Restart count: [0x01] (1 restart)
295+
blob := []byte{0x05, 0x80, 0x00, 0x00, 0x01}
296+
297+
br, err := newBlockReader(blob)
298+
if err != nil {
299+
return // parseIndexBlock caught it
300+
}
301+
// Reading value 4 should find 5, then try to iterate and hit truncated varint
302+
// Actually, this will find 5 as the answer without hitting truncation.
303+
// We need to search for value >= 5 to force iteration
304+
_, err = br.readGreaterThan(5)
305+
if err == nil {
306+
t.Fatal("Expected error when reading truncated varint in loop, got nil")
307+
}
308+
})
309+
310+
t.Run("overflow varint at restart", func(t *testing.T) {
311+
// Create a block with varint that would overflow uint64
312+
// 10 bytes of 0xFF creates an overflow (more than 64 bits)
313+
overflowVarint := []byte{0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x7F}
314+
// Restart points to this overflow varint
315+
blob := append(overflowVarint, 0x00, 0x00, 0x01) // restart at 0, count 1
316+
317+
br, err := newBlockReader(blob)
318+
if err != nil {
319+
return // parseIndexBlock might catch it
320+
}
321+
// Try to read should hit overflow varint
322+
_, err = br.readGreaterThan(100)
323+
if err == nil {
324+
t.Fatal("Expected error when reading overflow varint, got nil")
325+
}
326+
})
327+
}
328+
329+
// TestBlockWriterCorruptedVarint tests that scanSection properly handles
330+
// corrupted varint encoding by breaking out of the loop instead of hanging.
331+
func TestBlockWriterCorruptedVarint(t *testing.T) {
332+
// Create a valid block
333+
elements := []uint64{1, 5, 10, 11, 20, 100, 200, 300}
334+
bw, _ := newBlockWriter(nil, newIndexBlockDesc(0))
335+
for i := 0; i < len(elements); i++ {
336+
bw.append(elements[i])
337+
}
338+
validBlob := bw.finish()
339+
340+
// Create a new writer with corrupted data
341+
desc := &indexBlockDesc{
342+
id: 0,
343+
max: 300,
344+
entries: uint16(len(elements)),
345+
}
346+
347+
// Corrupt the blob
348+
blob := slices.Clone(validBlob)
349+
restarts, data, _ := parseIndexBlock(blob)
350+
if len(restarts) > 0 {
351+
// Corrupt data in the first section
352+
pos := int(restarts[0]) + 1 // Skip first valid varint
353+
if pos < len(data)-10 {
354+
for i := 0; i < 10; i++ {
355+
data[pos+i] = 0xFF // Invalid varint
356+
}
357+
}
358+
}
359+
360+
bw, err := newBlockWriter(blob, desc)
361+
if err != nil {
362+
// parseIndexBlock caught the corruption, which is fine
363+
return
364+
}
365+
366+
// Test scanSection with corrupted data - should break instead of hanging
367+
callbackCalled := false
368+
bw.scanSection(0, func(v uint64, pos int) bool {
369+
callbackCalled = true
370+
return false // Continue iteration to test the corrupted part
371+
})
372+
// The callback should have been called at least once for the first valid element
373+
// before hitting corruption and breaking
374+
if !callbackCalled {
375+
t.Log("scanSection didn't call callback - may have hit corruption immediately")
376+
}
377+
378+
// Test sectionLast with corrupted data - should handle gracefully
379+
_ = bw.sectionLast(0)
380+
381+
// If we got here without hanging or panicking, the test passes
382+
}

0 commit comments

Comments
 (0)