cockroachdb / cockroachdb/cockroach
storage: nil-pointer dereference in BackupCompactionIterator.assertInvariants on exhausted iterator
- 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
Assessment
This issue has not been assessed yet.