cockroachdb / cockroachdb/cockroach

storage: nil-pointer dereference in BackupCompactionIterator.assertInvariants on exhausted iterator

Open
#166,619 1 comment 0 reactions 0 assignees View on GitHub
branch-master C-bug O-agent T-testeng
Dominant language
Go
Stars
32.5k
Forks
4.1k
PR merge metrics
PR metrics pending

Description

## Summary

`BackupCompactionIterator.assertInvariants()` (and the identical code in `ReadAsOfIterator.assertInvariants()`) contains a latent nil-pointer dereference. When the underlying iterator's `Valid()` returns `(false, nil)` — meaning exhausted with no error — the code unconditionally calls `err.Error()` on a nil `err`, which would panic.

## Affected Code

**`pkg/storage/backup_compaction_iterator.go:144-146`:**
```go
if ok, err := f.iter.Valid(); !ok || err != nil {
errMsg := err.Error() // PANIC if err == nil && !ok
return errors.AssertionFailedf("invalid underlying iter with err=%s", errMsg)
}
```

**`pkg/storage/read_as_of_iterator.go:247-249`:** identical pattern.

## Trigger Condition

The code is only reachable in race builds (`util.RaceEnabled`). Under normal single-threaded operation the state is unreachable, but the code is defensively incorrect.

## Suggested Fix

```go
if ok, err := f.iter.Valid(); !ok || err != nil {
if err != nil {
return errors.AssertionFailedf("invalid underlying iter with err=%s", err.Error())
}
return errors.AssertionFailedf("underlying iter unexpectedly invalid without error")
}
```

Apply the same fix to both `BackupCompactionIterator` and `ReadAsOfIterator`.

_This issue was found via automated deep static analysis._

Jira issue: CRDB-62031

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.