Skip to content

Fix critical DPoP nonce bypass and security issues#8

Open
Cache8063 wants to merge 2 commits intostreamplace:mainfrom
Cache8063:fix/security-critical-findings
Open

Fix critical DPoP nonce bypass and security issues#8
Cache8063 wants to merge 2 commits intostreamplace:mainfrom
Cache8063:fix/security-critical-findings

Conversation

@Cache8063
Copy link

Summary

  • Fix DPoP nonce bypass in DPoPNonceMiddleware — the middleware was validating its own response header instead of the client's DPoP JWT nonce, meaning nonce validation always passed regardless of what the client sent
  • Add missing error checks after GetOauthClient() in Return() and getOAuthSession() where errors were silently overwritten by the next := assignment
  • Remove access token from error log in Return() to prevent credential leakage in log output
  • Remove debug print statements (fmt.Println / fmt.Printf) that leaked session data to stdout

Ref: #7

Details

DPoP Nonce Bypass (Critical)

In DPoPNonceMiddleware, the code was:

validNonces := generateValidNonces(...)
c.Response().Header().Set("DPoP-Nonce", validNonces[0])  // set on response
dpopNonce := c.Response().Header().Get("DPoP-Nonce")      // read back own value
if !slices.Contains(validNonces, dpopNonce) { ... }        // always passes

Fixed to validate the nonce from the client's DPoP JWT claims (dpopClaims.Nonce), matching the correct pattern already used in OAuthMiddleware.

Unchecked Errors

GetOauthClient() returns (*Client, error) but the error was being silently overwritten:

oclient, err := o.GetOauthClient()
jkt, _, err := parseState(state)  // overwrites err

Added proper error checks before proceeding.

Test plan

  • go build ./... passes
  • go vet ./... passes
  • go test ./... passes
  • Manual verification of DPoP nonce flow with a real client

🤖 Generated with Claude Code

bkb and others added 2 commits March 14, 2026 17:20
- Fix DPoP nonce bypass in DPoPNonceMiddleware: validate the client's
  nonce from the DPoP JWT claims instead of reading back our own
  response header (which always passed validation)
- Add error checks after GetOauthClient() in Return() and
  getOAuthSession() — errors were silently overwritten by the next
  variable assignment
- Remove access token from error log in Return() to prevent credential
  leakage in log output
- Remove debug fmt.Println in HandleOAuthToken and fmt.Printf in
  Status() that leaked session data to stdout

Ref: streamplace#7

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- mattn/go-sqlite3 v1.14.22 → v1.14.34 (CVE-2025-6965: CRITICAL SQLite
  memory corruption, CVE-2025-29087: concat_ws overflow)
- golang.org/x/crypto v0.39.0 → v0.49.0 (CVE-2025-58181: SSH GSSAPI DoS)
- golang.org/x/net v0.41.0 → v0.52.0 (CVE-2025-47911, CVE-2025-58190:
  html parse DoS)
- golang.org/x/sys v0.33.0 → v0.42.0
- golang.org/x/text v0.26.0 → v0.35.0
- Go minimum version bumped to 1.25.0 (required by x/crypto v0.49.0)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant