elastic / elastic/beats

[packetbeat-parser-safety] Redis RESP array count can trigger makeslice panic in parseArray

Open Beginner friendly
#51,143 1 comment 0 reactions 0 assignees View on GitHub
needs_team
Dominant language
Go
Stars
12.7k
Forks
5k
Avg merge
2d 54m
Merged PRs (30d)
381

Description

## Findings

### 1. Redis `parseArray` panics on malformed huge array count

**Severity:** High

**Location:**
- `packetbeat/protos/redis/redis_parse.go:383-405`
- `packetbeat/protos/redis/redis_parse.go:411`

**Evidence:**
- The parser reads an untrusted array count from packet bytes and uses it as a `make` capacity:
- `count, err := parseInt(line[1:])` (`redis_parse.go:383`)
- `content = make([][]byte, 0, count)` (`redis_parse.go:404`)
- For malformed input `*9223372036854775807\r\n`, this panics with runtime error:
- `panic: runtime error: makeslice: cap out of range`
- stack top: `packetbeat/protos/redis/redis_parse.go:404`

**What is wrong:**
A packet-controlled length field is used directly as an allocation bound without validating it against sane limits or `int`/buffer constraints.

**Why it matters:**
Any peer that can send malformed RESP to Packetbeat can crash parser execution (DoS) via a single oversized array header, before message parsing can fail gracefully.

**Suggested fix:**
Validate `count` before allocation and before loop conversion, e.g. reject values that are too large for `int`, exceed configured protocol maximums, or are impossible relative to the remaining buffer.

## Paste-ready reproducer test

Add to `packetbeat/protos/redis/redis_test.go`:

```go
func TestRedisParser_NoPanic_OnHugeArrayCount(t *testing.T) {
assert.NotPanics(t, func() {
_, _, _ = parse([]byte("*9223372036854775807\r\n"))
}, "parser must reject malformed huge array count without panic")
}
```

Running this test currently fails with:

```text
panic: runtime error: makeslice: cap out of range
...
packetbeat/protos/redis.(*parser).parseArray(...)
packetbeat/protos/redis/redis_parse.go:404
```

## Unsafe-location checklist (grouped by parser)

- [ ] **redis**: `packetbeat/protos/redis/redis_parse.go:404` uses untrusted `count` from `redis_parse.go:383` as `make` capacity.
- [ ] **redis**: `packetbeat/protos/redis/redis_parse.go:411` converts `count` to `int` for loop bound; guard this after validating `count` range.

## Coverage this run

Inspected and cross-checked high-severity parser-safety candidates in:
`amqp`, `cassandra`, `dns`, `http`, `icmp`, `memcache`, `mongodb`, `mysql`, `nfs`, `pgsql`, `redis`, `sip`, `thrift`, `tls`.

Previously reported open findings were not re-filed (`#51040`, `#50325`, `#50205`, `#49941`, `#49767`).

---
[What is this?](https://ela.st/github-ai-tools) | [From workflow: Sweeper: Packetbeat Parser Bounds Safety](https://github.com/elastic/beats/actions/runs/27132224644)

Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
> - [x] expires on Jun 15, 2026, 10:57 AM UTC

Contributor guide

Open the contributing guide

Research direction

Start in packetbeat/protos/redis/redis_parse.go at parseArray, especially the count parsing, allocation, and loop around lines 383-411. Add the regression test from the issue to packetbeat/protos/redis/redis_test.go and run it; done means the huge RESP array count is rejected without a panic and the test passes.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
networking
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
74/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.