Skip to content

Security: PKCE not enforced on public OAuth clients #31

Description

@Freedisch

Summary

PKCE verification in POST /oauth/token is silently skipped if no code_challenge was stored during authorization. For public clients like Claude (no client secret), PKCE is the only protection against auth code interception — it should be required, not optional.

Affected code

havril/internal/api/handlers/oauth_handler.goToken

// Skipped entirely if the client didn't send code_challenge
if entry.codeChallenge != "" {
    verifier := r.FormValue("code_verifier")
    if !verifyS256(verifier, entry.codeChallenge) {
        h.tokenError(w, "invalid_grant", http.StatusBadRequest)
        return
    }
}

Why this matters

Without PKCE, if an attacker intercepts the auth code in the redirect (e.g. via a browser history leak, referrer header, or malicious browser extension), they can exchange it for a bearer token at /oauth/token with no further proof of identity. PKCE prevents this by requiring knowledge of the original code_verifier that only the legitimate client holds.

Proposed fix

Reject the authorization request upfront if code_challenge is absent or not S256:

// in authorizeSubmit and autoAuthorize, before issueCode
if q.Get("code_challenge") == "" || q.Get("code_challenge_method") != "S256" {
    http.Error(w, "PKCE with S256 is required", http.StatusBadRequest)
    return
}

This forces all clients to use PKCE rather than making it opt-in.

Severity

Medium — Claude always sends PKCE so this doesn't affect the current integration, but any future MCP client that skips PKCE would be vulnerable.

Metadata

Metadata

Assignees

No one assigned

    Labels

    help wantedExtra attention is needed

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions