Skip to content
Hwat Sauce avatarhwat.sauce
~/posts/how-i-found-my-first-cve-in-go-redis.mdx

$ cat how-i-found-my-first-cve-in-go-redis.mdx

_

$ date → July 10, 202610 min read1,996 words

How I Found and Fixed a Panic Bug in go-redis by Fuzzing the RESP3 Parser

A first-timer's walkthrough of fuzzing the RESP3 reply parser in go-redis — from a five-second crash to disclosure with Redis's security team and a four-line fix merged into a library with millions of downloads.


I want to start by saying I did not expect any of this to go the way it did. I picked go-redis as a learning exercise. I thought I would run some fuzz tests, find nothing, and move on. Instead, five seconds into the first fuzz run, the test suite exploded with a panic I had never seen before. What followed was two weeks of debugging, emails with the Redis security team, a minor disagreement with a senior engineer, and eventually a merged PR that ships to every go-redis user who updates.

This is the full story.

Where It Started#

I had been reading about bug bounty research and specifically about the "spec to code" bug class. I wanted to learn the methodology but I did not want to jump straight into complex codegen territory. I needed a simpler target to learn the workflow first. I was already familiar with Go and I had been reading about OSS-Fuzz, so I decided to look at open source Go libraries that processed some kind of external input.

I landed on go-redis for a specific reason: it is a Redis client library, and Redis clients have to parse a server protocol. The server is supposed to be trusted, but that is exactly the kind of assumption that makes parsers sloppy. And a go-redis v9 client uses RESP3 by default, which is a newer, more complex protocol with types like maps, sets, doubles, booleans, and push notifications that RESP2 never had. New protocol types in a mature codebase is historically where gaps show up.

Reading the Build File#

The first thing I did was look at how OSS-Fuzz integrated go-redis. The integration is a single line:

compile_go_fuzzer github.com/redis/go-redis/v9/fuzz Fuzz fuzz gofuzz
bash
compile_go_fuzzer github.com/redis/go-redis/v9/fuzz Fuzz fuzz gofuzz
bash

That tells you the harness lives in go-redis/fuzz/fuzz.go, the entry function is Fuzz, and it uses the old go-fuzz interface. When I actually read the harness, I found the problem immediately. The harness tried to connect to a live Redis server at :6379, ran a few commands against it, and threw away all the errors. Without a server, every call just returned a connection error. The fuzzer was mutating the bytes that became command keys and values, but those bytes never reached anything interesting because they never got to a server.

More importantly, the harness was pointed at the wrong direction entirely. A Redis client library's trust boundary is not the commands it sends. It is the replies it receives. An attacker does not control what commands a client sends. But if they compromise the server or intercept the network connection, they control every byte the client reads back. The harness was fuzzing the write path. The real attack surface was the read path.

Writing a Better Harness#

The RESP3 reply parser lives in internal/proto/reader.go. The entry point is ReadReply(), which reads a byte, figures out the reply type from the leading character, and dispatches accordingly. The full grammar includes simple strings, errors, integers, bulk strings, arrays, maps, sets, doubles, booleans, big numbers, verbatim strings, null, and push notifications.

Because I was working inside the module's own directory, I could import internal/proto without any restriction. I placed the fuzz test inside internal/proto/reader_fuzz_test.go:

package proto
 
import (
    "bytes"
    "testing"
)
 
func FuzzReadReply(f *testing.F) {
    for _, s := range []string{
        "+OK\r\n",
        ":123\r\n",
        "-ERR x\r\n",
        "$5\r\nhello\r\n",
        "*2\r\n$3\r\nfoo\r\n$3\r\nbar\r\n",
        "_\r\n",
        "#t\r\n",
        ",3.14\r\n",
        "%1\r\n+k\r\n+v\r\n",
        "~2\r\n+a\r\n+b\r\n",
        ">2\r\n+pub\r\n+msg\r\n",
    } {
        f.Add([]byte(s))
    }
    f.Fuzz(func(t *testing.T, data []byte) {
        r := NewReader(bytes.NewReader(data))
        _, _ = r.ReadReply()
    })
}
go
package proto
 
import (
    "bytes"
    "testing"
)
 
func FuzzReadReply(f *testing.F) {
    for _, s := range []string{
        "+OK\r\n",
        ":123\r\n",
        "-ERR x\r\n",
        "$5\r\nhello\r\n",
        "*2\r\n$3\r\nfoo\r\n$3\r\nbar\r\n",
        "_\r\n",
        "#t\r\n",
        ",3.14\r\n",
        "%1\r\n+k\r\n+v\r\n",
        "~2\r\n+a\r\n+b\r\n",
        ">2\r\n+pub\r\n+msg\r\n",
    } {
        f.Add([]byte(s))
    }
    f.Fuzz(func(t *testing.T, data []byte) {
        r := NewReader(bytes.NewReader(data))
        _, _ = r.ReadReply()
    })
}
go

The seed corpus covers all the RESP3 type prefixes. The harness itself is three lines. Feed bytes in, call ReadReply, ignore the result. A panic or a hang is the finding.

Five Seconds#

I ran it:

cd internal/proto
go test -fuzz=FuzzReadReply -fuzztime=10m
bash
cd internal/proto
go test -fuzz=FuzzReadReply -fuzztime=10m
bash

Five seconds later:

--- FAIL: FuzzReadReply (5.42s)
    panic: runtime error: hash of unhashable type map[interface {}]interface {}
        github.com/redis/go-redis/v9/internal/proto.(*Reader).readMap(...)
        ~/go-redis/internal/proto/reader.go:455 +0x3d4
--- FAIL: FuzzReadReply (5.42s)
    panic: runtime error: hash of unhashable type map[interface {}]interface {}
        github.com/redis/go-redis/v9/internal/proto.(*Reader).readMap(...)
        ~/go-redis/internal/proto/reader.go:455 +0x3d4

I stared at this for a moment. The fuzzer had found a crashing input in five seconds on native Go tooling running natively on Apple Silicon. The crash was in readMap, which is the function that handles RESP3 map replies.

The minimized input the fuzzer saved was:

%1\r\n%0\r\n+\r\n
%1\r\n%0\r\n+\r\n

Parsing that out: %1 is a RESP3 map header with one key-value pair. The key is %0, which is an empty RESP3 map. The value is +\r\n, which is an empty simple string. When readMap tried to do m[k] = v where k was itself a map[interface{}]interface{}, Go's runtime panicked because maps are not hashable and cannot be used as map keys.

The bug was in reader.go at line 455, last touched in 2021:

m[k] = v  // panics if k is a map or slice
go
m[k] = v  // panics if k is a map or slice
go

No type check. No guard. Just raw assignment.

Building the Reachability PoC#

Finding a panic in an internal package test is interesting but not enough. The question maintainers always ask is: can this be reached through the actual public API? I needed to show that a real redis.Client talking to a real server would crash on this input.

I wrote a mock TCP server in Go that handled the RESP3 handshake properly. The real complexity was that go-redis sends several commands during connection setup including a HELLO negotiation and CLIENT SETINFO for library metadata. My first attempt at a static handshake reply was wrong and the client timed out waiting for a reply that never came. The second version actually parsed incoming RESP requests and replied correctly to each handshake command, then fired the malicious reply when it saw the real target command arrive.

With rdb.Do(ctx, "NOSUCHCMD").Result(), the generic Cmd type went through the untyped ReadReply dispatcher rather than a typed parser. The panic landed:

panic: runtime error: hash of unhashable type map[interface {}]interface {}
    proto.(*Reader).readMap(...)        reader.go:455
    proto.(*Reader).ReadReply(...)      reader.go:345
    redis.(*Cmd).readReply(...)         command.go:754
    redis.(*baseClient)._process(...)   redis.go:996
    redis.(*Client).Process(...)        redis.go:1472
    main.main()                         poc/poc.go:108
panic: runtime error: hash of unhashable type map[interface {}]interface {}
    proto.(*Reader).readMap(...)        reader.go:455
    proto.(*Reader).ReadReply(...)      reader.go:345
    redis.(*Cmd).readReply(...)         command.go:754
    redis.(*baseClient)._process(...)   redis.go:996
    redis.(*Client).Process(...)        redis.go:1472
    main.main()                         poc/poc.go:108

The full call chain from main() to the crash site, all through public API. That was the proof.

The Disclosure#

go-redis has no SECURITY.md but their CONTRIBUTING.md said to email oss@redis.com for security issues. I wrote up the finding with the minimized reproducer, the PoC code, the suggested fix, and a calibrated severity (DoS, not RCE) and sent it.

The response came back in about twenty minutes. David Maier acknowledged it. Yonathan Levy asked internally whether a VDP ticket existed for the report. Curtis Means, Senior Product Security Engineer, replied that he had checked all three email addresses and the HackerOne platform and could not find a matching ticket, and asked how they received the report.

That last email was actually good news. It meant a senior engineer was personally tracking it down rather than leaving it in a queue. I replied explaining I had sent it directly to oss@redis.com per the contributing guide, and named the files I had attached.

The Debate#

Nedyalko Dyakov wrote back with a thorough technical response. He confirmed the bug was real and agreed on the fix. But he said they did not consider it a security vulnerability and would prefer to handle it as a normal bug rather than a security advisory. His reasoning was that a Redis client trusts the server it authenticates to. To trigger this panic, an attacker needs to already control the server or intercept the transport. At that point, crashing the client is the least of the victim's problems. He specifically pushed back on my go-yaml comparison: go-yaml parses arbitrary user documents, so any user input can trigger the panic there. In go-redis, only an already-compromised transport can deliver the malicious frame.

I thought about this for a while. His argument is genuinely good. The threat model distinction between a parser that accepts arbitrary documents and a protocol client that speaks to an authenticated peer is real. I was not going to pretend otherwise.

But I did push back on one thing. Defensive input handling is valuable even when the source is supposed to be trusted. Network assumptions get violated in ways that are hard to predict. Proxies, chaos testing frameworks, debugging tools, network interception for legitimate purposes. A parser that panics on malformed input is worse than one that returns a clean error, regardless of how malformed input gets there. And the fix is four lines.

Ned's reply was direct: "Agreed on the fix. A parser shouldn't panic on malformed bytes regardless of source, so please go ahead." He also sharpened the go-yaml distinction: YAML's input contract is untrusted documents, Redis's is an authenticated peer. Same bug class, different reachability. He was drawing the line at classification, not at whether to fix it. And he asked me to open a public issue and send the PR.

I appreciated that response more than if he had just capitulated immediately. He explained his reasoning, I explained mine, and we found the real point of agreement. That is a good technical discussion.

The Fix and the PR#

The fix itself is small. In readMap, before m[k] = v, check whether k is an unhashable type and return a parse error instead:

switch k.(type) {
case []interface{}, map[interface{}]interface{}:
    return nil, fmt.Errorf("redis: RESP3 map key must be a scalar type, got %T", k)
}
m[k] = v
go
switch k.(type) {
case []interface{}, map[interface{}]interface{}:
    return nil, fmt.Errorf("redis: RESP3 map key must be a scalar type, got %T", k)
}
m[k] = v
go

This converts an unrecovered runtime panic into a clean error that callers can handle through the normal error path. Legitimate servers never send aggregate-keyed maps, so this guard never trips on valid input.

I forked the repo, created a branch called fix/resp3-map-key-panic, applied the fix, added a regression test that feeds the crashing input and asserts an error return, and opened the PR.

A Speed Bump#

Within a few minutes, a bot called Cursor Bugbot left a code review comment on the PR. It had found a real issue:

"The new RESP3 map key scalar check runs only after a successful value read. When the value is Nil or a RedisError, readMap still writes m[k] and continues before that check, so slice or map keys can still panic instead of returning the new parse error."

This was a legitimate catch. My fix was in the right place for the happy path but the early-exit branches for Nil and error values bypassed it. The fix needed to move earlier in the function, before any of the conditional branches that write to the map. I updated the implementation and pushed a follow-up commit.

Merged#

A maintainer merged the master branch into my PR branch to bring it up to date. A few days later:

Pull request successfully merged and closed.
Pull request successfully merged and closed.

Done. The fix shipped. Every go-redis user who runs go get github.com/redis/go-redis/v9@latest gets it.

What I Learned#

A few things stuck with me from this whole experience.

The existing harness was the problem. OSS-Fuzz had been running go-redis for years with a harness that required a live server and pointed at the wrong trust boundary. Improving a harness can be as valuable as finding a bug. If a fuzzer is not reaching the code that matters, it is not finding what matters.

The right target for a client library is the read path. A client controls what it sends. It does not control what it receives. The attack surface is always the reply parser, not the command builder.

The Cursor Bugbot catch was humbling and good. My first fix had a real gap in it. Automated review caught it before the maintainers had to. Running your own code review tools before submitting a PR is worth the time.

Classification matters less than the fix. Whether this becomes a CVE or not, the code is better. A parser that handles malformed input gracefully is more correct than one that panics. That was always the real goal.


The merged PR is at github.com/redis/go-redis/pull/3873. The fix is in internal/proto/reader.go. The regression test is TestReadMapRejectsUnhashableKey in reader_fuzz_test.go.

Stay in the loop with my latest content – follow me on Medium for more!

THANKS#

MUHAMMAD ABDULLAH#