pkg/lisafs: PReadResp.CheckedUnmarshal panics on malformed NumBytes due to uint32 truncation in bounds check
- Dominant language
- Go
- Stars
- 19.3k
- Forks
- 2k
- Avg merge
- 3d 5h
- Merged PRs (30d)
- 264
Description
# [Bug] pkg/lisafs: PReadResp.CheckedUnmarshal panics on malformed NumBytes (uint32 truncation in bounds check)
## Summary
`PReadResp.CheckedUnmarshal` validates `NumBytes` using **uint32-truncated** comparisons but slices `Buf` using the **full uint64** value. Any response frame whose 8-byte NumBytes has nonzero high bits passes validation and then panics with `slice bounds out of range`.
## Affected
`pkg/lisafs/message.go`, `func (r *PReadResp) CheckedUnmarshal` (~line 913):
```go
srcRemain, ok := r.NumBytes.CheckedUnmarshal(src)
if !ok || uint32(r.NumBytes) > uint32(len(srcRemain)) || uint32(r.NumBytes) > uint32(len(r.Buf)) {
return src, false
}
r.Buf = r.Buf[:r.NumBytes] // full uint64 used here
```
`uint32(0x0000000C00000000) == 0`, so all checks pass and the slice panics.
## Repro
```go
func TestReproPReadRespPanic(t *testing.T) {
in := []byte("\x00\x00\x00\x00\x0c\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00")
var m PReadResp
defer func() {
if r := recover(); r != nil {
t.Fatalf("CONFIRMED PANIC: %v", r)
}
}()
m.CheckedUnmarshal(in)
}
```
Observed: `panic: runtime error: slice bounds out of range [:51539607552] with capacity 0`
## Suggested fix
Compare at full width before slicing:
```go
if !ok || r.NumBytes > uint64(len(srcRemain)) || r.NumBytes > uint64(len(r.Buf)) {
return src, false
}
```
## Audit note
A randomized sweep of all other lisafs message `CheckedUnmarshal` implementations (8M structured mutations) found no additional panics; the visually-similar `PWriteReq.CheckedUnmarshal` guard is not affected because its NumBytes is consumed as a 32-bit quantity on this path.
## Impact context
PReadResp is parsed client-side (Sentry) from gofer responses. A malformed frame reaching this parser crashes the Sentry process hosting the sandbox's filesystem view. While gofer responses are host-generated, defensive completeness in "Checked"Unmarshal implementations is what lets the rest of lisafs treat parsing as infallible; this instance breaks that contract.
Contributor guide
Research direction
Start in pkg/lisafs/message.go at PReadResp.CheckedUnmarshal, then run the TestReproPReadRespPanic example from the issue with the malformed NumBytes frame. Ensure malformed high-bit values are rejected without a panic, while valid responses still parse successfully; add or update focused tests near the existing lisafs message tests.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- operating-systems
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 88/100