Skip to content

security: malformed Cookie metadata can panic the control plane #7216

Description

@vanshika2720

Problem

extractCookie in pkg/rpc/rpcauth/auth.go parses the incoming gRPC cookie metadata by splitting on ; and then, for each segment, splitting on = with strings.SplitN(..., "=", 2). It then unconditionally indexes subs[1]:

cs := strings.Split(rawCookie[0], ";")
cookie := make(map[string]string, len(cs))
for _, c := range cs {
	subs := strings.SplitN(strings.TrimSpace(c), "=", 2)
	cookie[subs[0]] = subs[1]
}

If any ;-separated segment does not contain an = (for example a trailing ;, a bare cookie name, or two consecutive ;;), strings.SplitN returns a slice of length 1, and subs[1] panics with index out of range [1] with length 1.

Impact

This is reachable before authentication succeeds. extractCookie is called as the very first step of JWTUnaryServerInterceptor (pkg/rpc/rpcauth/interceptor.go:189), i.e. before the JWT itself is verified:

func JWTUnaryServerInterceptor(verifier jwt.Verifier, authorizer RBACAuthorizer, logger *zap.Logger) grpc.UnaryServerInterceptor {
	return func(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) {
		cookie, err := extractCookie(ctx)
		...

This interceptor is installed on the WebAPI gRPC server via rpc.WithJWTAuthUnaryInterceptor in cmd/controlplane/server.go:309. So any unauthenticated caller that can reach the WebAPI gRPC port and send a request with a malformed cookie metadata value (no valid session/JWT required) can trigger the panic on any WebAPI RPC method.

Each gRPC request is served in its own goroutine spawned directly by grpc-go (go f() in (*Server).serveStreams, see google.golang.org/grpc@v1.79.3/server.go), with no recover() anywhere in that call path. I confirmed there is no panic-recovery interceptor registered anywhere in pkg/rpc/ or pkg/app/server/ (grep -r "recover()" and searches for grpc_recovery/recovery interceptors both come back empty). An unrecovered panic in any goroutine terminates the entire Go process.

The WebAPI, PipedAPI, and APIService gRPC servers all run inside the same controlplane OS process, started from the same errgroup in cmd/controlplane/server.go (server.run). So a single malformed, unauthenticated request to the WebAPI port crashes the whole control-plane process, not just the WebAPI listener — taking down piped↔control-plane communication (PipedAPI) and the external API (APIService) as well until the process is restarted.

Technical Details

  • Vulnerable code: extractCookie, pkg/rpc/rpcauth/auth.go:127-143, specifically the indexing at line 140 (cookie[subs[0]] = subs[1]).
  • Call path: unauthenticated gRPC request → JWTUnaryServerInterceptor (pkg/rpc/rpcauth/interceptor.go:189) → extractCookie (auth.go:136-141) → panic on subs[1].
  • No bounds check exists on len(subs) before the indexing.
  • No panic-recovery interceptor is registered on the WebAPI server (or any other gRPC server) in this codebase, and grpc-go itself does not recover panics from handler/interceptor goroutines.
  • WebAPI, PipedAPI, and APIService all run in the same OS process (cmd/controlplane), so the crash is process-wide, not limited to the WebAPI listener.

Regression

This is a regression introduced by #6933 ("Update extract cookie logic to allow cookie contains equal sign"). Before that PR, the code was:

subs := strings.Split(strings.TrimSpace(c), "=")
if len(subs) != 2 {
	return nil, status.Error(codes.Unauthenticated, "cookie is malformed")
}
cookie[subs[0]] = subs[1]

#6933 changed strings.Split to strings.SplitN(..., "=", 2) (to correctly support cookie values that themselves contain =, e.g. base64 padding) but removed the len(subs) != 2 bounds check entirely, so the fix for one bug reintroduced a panic for a different malformed-input case. There is currently no test case covering a cookie segment without = (pkg/rpc/rpcauth/auth_test.go's TestExtractCookie only covers well-formed key=value segments and the "value contains =" case added by #6933).

Reproduction

Locally reproduced with a temporary test against extractCookie (not committed):

ctx := metadata.NewIncomingContext(context.Background(), metadata.MD{
	"cookie": []string{"token=xxx;"},
})
_, _ = extractCookie(ctx)

Result:

panic: runtime error: index out of range [1] with length 1

github.com/pipe-cd/pipecd/pkg/rpc/rpcauth.extractCookie(...)
	pkg/rpc/rpcauth/auth.go:140

The same panic is triggered by any segment lacking =, e.g. Cookie: justatoken or Cookie: token=xxx;;.

Equivalently, over the wire, sending any WebAPI RPC with gRPC metadata cookie: token=xxx; (no valid JWT required, since the panic happens before verification) is expected to crash the control-plane process.

Related Issues

#6612 requests a generic panic-recovery interceptor for the server ("Server should not crash when panic occurs") and is already being worked on. It would provide useful defense-in-depth against panics like this one, but it does not identify or document this specific extractCookie trigger, and adding generic recovery does not address the underlying missing bounds check that lets unauthenticated input reach an out-of-range index in the first place. This issue is about fixing that specific input-validation regression; #6612 is a separate, complementary hardening effort.

Suggested Fix

Restore bounds validation before indexing the result of SplitN in extractCookie, e.g. verify len(subs) == 2 before accessing subs[1], and reject/skip malformed segments instead of indexing them unconditionally. This should be done without regressing the fix from #6933 (i.e. cookie values containing = should still be supported).

Separately, landing #6612's generic panic-recovery interceptor would add useful defense-in-depth for this class of bug, but should not be treated as a substitute for fixing this specific unauthenticated-input-handling issue.

Priority

This repository doesn't have a P0 label; the closest equivalent is priority/P1 ("Top priority"). I believe this qualifies:

  • Triggerable by an unauthenticated request (no valid credentials/JWT needed — the panic occurs before verification).
  • Requires only a single, trivially crafted gRPC metadata value.
  • Confirmed to crash the entire controlplane process (WebAPI, PipedAPI, and APIService all run in one process with no panic recovery anywhere in the call path), not just the affected request.
  • It's a regression in currently-released code (Update extract cookie logic to allow cookie contains equal sign #6933), not a theoretical/unreached code path.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions