{"uuid": "13355ca2-6ec8-4ec5-8dae-2e11131923e5", "vulnerability_lookup_origin": "1a89b78e-f703-45f3-bb86-59eb712668bd", "author": "9f56dd64-161d-43a6-b9c3-555944290a09", "vulnerability": "GHSA-qgp8-v765-qxx9", "type": "seen", "source": "https://gist.github.com/MarisollieNULL/935455fadca48e7a3343eb0015eb0b69", "content": "# Read the Issue Tracker Before You Read the Source\n\n_A pre-mortem discipline for auditing OSS libraries, anchored to two findings that died on five minutes of public-record reading I should have done up front._\n\n## The setup\n\nI was auditing [`cloudflare/workers-oauth-provider`](https://github.com/cloudflare/workers-oauth-provider) with an OAuth/OIDC specialization lens. Earlier sessions had only looked at the consent-render template path, so the remaining ~3000 LOC token-grant pipeline (`oauth-provider.ts:1827-2717`) had never been walked under the OAuth state-machine lens.\n\nSo I walked it. The walk produced two source-level findings that looked filable on first read. Both turned out to be already-public knowledge, not in some obscure mailing list but in the library's own GitHub issue tracker and security advisories. A five-minute dup-check at the start of the audit would have killed both before I spent ninety minutes confirming them.\n\nThis is the methodology lesson I should have learned earlier. Writing it down so I (and anyone else with the same blind spot) stop making it.\n\n## What I found\n\n### Finding 1, plain PKCE method enabled by default\n\nIn [`src/oauth-provider.ts:1778`](https://github.com/cloudflare/workers-oauth-provider/blob/main/src/oauth-provider.ts):\n\n```ts\ncode_challenge_methods_supported:\n  this.options.allowPlainPKCE !== false ? ['plain', 'S256'] : ['S256']\n```\n\nThe PKCE verification at lines 1957-1967 accepts `plain` via direct compare. OAuth 2.1 \u00a77.6 requires S256, so allowing `plain` is a spec violation unless the operator explicitly opts into S256-only.\n\nI ran live discovery probes across four deployed Cloudflare MCP servers (`radar.mcp.cloudflare.com`, `dns-analytics.mcp.cloudflare.com`, `ai-gateway.mcp.cloudflare.com`, `autorag.mcp.cloudflare.com`). All four advertised `code_challenge_methods_supported: [\"plain\", \"S256\"]` at `/.well-known/oauth-authorization-server`. Source matched deployment.\n\nLooked filable. Time to file?\n\n### Finding 2, refresh token rotation reuse-detection gap\n\nIn `src/oauth-provider.ts:2353-2362`, right above the refresh-grant state update, the library ships this comment:\n\n> // The token which the client used this time becomes the \"previous\" token,\n> // so that the client can always use the same token again next time. This\n> // might technically violate OAuth 2.1's requirement that refresh tokens be\n> // single-use. However, this requirement violates the laws of distributed systems...\n\nThe state-machine confirms it. Line 2363-2364 sets `previousRefreshTokenId = providedTokenHash`, so an attacker holding a stolen `RT1` can keep refreshing forever. The server rotates current to new but pins previous to `RT1`, overwritten with the same value on each request. A legitimate user with a cached-but-stale `RT2` then gets `invalid_grant` and is locked out.\n\nTextbook RFC 6749 \u00a710.4 and OAuth 2.1 \u00a76.1 violation, and the in-code comment is honest about it.\n\nLooked filable. Time to file?\n\n## Why neither pays\n\nI ran the five-minute public-record dup-check after the source-walk, and both findings collapsed.\n\nFor finding 1 (PKCE), commit [`dbb150ed`](https://github.com/cloudflare/workers-oauth-provider/commit/dbb150ed) dated 2026-02-26 says _\"Add allowPlainPKCE option to enforce S256 PKCE (OAuth 2.1) (#151)\"_. Cloudflare explicitly added the safe-config option and chose to default to permissive. A separate earlier issue (GHSA-qgp8-v765-qxx9, 2025-05-01) covered a different PKCE-check-skip bypass and was patched in v0.0.5. In bug-bounty terms, this is operator-opt-in misconfig: the library lets operators choose the secure mode, the operators didn't choose it, the vendor isn't liable. Reports of this shape close as Informative or By Design. Zero payout.\n\nFor finding 2 (refresh-token rotation), [Issue #43](https://github.com/cloudflare/workers-oauth-provider/issues/43) _\"Allowing the previous refresh token makes RT rotation useless\"_ was filed by `neilmadden-teya` on 2025-06-06. Twelve months open. Zero maintainer responses. The behavior is fully described and a fix is proposed in the thread. The in-code comment four lines above the offending state assignment explicitly acknowledges the spec violation. Silence plus an in-code \"we know\" comment equals design choice, not vulnerability. Any report citing this gets closed with \"known, see #43, see source comment, by design.\" Zero payout.\n\n## The lesson\n\nA lot of bug-hunting methodologies bake in a \"devil's advocate\" or \"pre-mortem\" step at hypothesis time. You write down the most plausible reason your hypothesis will die, then proceed.\n\nThe common failure mode (mine, anyway) is treating that paragraph as a hedge. You wrote it, the hypothesis survived your own most-honest objection, you move on to the audit.\n\nBut sometimes the devil's-advocate paragraph is concretely testable. Not \"the triager might disagree with my severity,\" which is aspirational and untestable until the report lands, but \"this is probably already in the issue tracker,\" which is verifiable in five minutes via `gh api`.\n\nWhen the DA paragraph names a concretely-testable failure mode, the first move of the audit IS that test, not the source-walk you originally planned. The DA wasn't a hedge. It was a falsifiable prediction. Run it.\n\nOf the two findings above, my mid-session pre-mortem actually predicted dup-density risk in roughly those words. I source-walked anyway. The lesson isn't that the DA was wrong. It's that the DA was right and I didn't act on it.\n\n## The five-minute protocol\n\nBefore source-walking any public OSS library that has its own GitHub issue tracker, run these four checks in parallel. Total wall-clock, five to ten minutes.\n\n```bash\n# 1. Full Security Advisory history with severity + patched-version\ngh api repos///security-advisories?state=published\n\n# 2. Filed issues matching the bug-class keyword\ngh issue list -R / --search '' --state all\n\n# 3. Recent commits, since security-relevant changes often have telling messages\ngit log --oneline -50\n\n# 4. Read SECURITY.md / AGENTS.md / CLAUDE.md if present.\n#    Maintainers often document accepted trade-offs in plain English.\n```\n\nThree patterns are worth knowing once the data comes back. An open issue six or more months old with no maintainer response, on your exact bug, means maintainer-accepted trade-off. Silence is disposition. A commit that adds an opt-in safe-config flag (think `allowPlainPKCE`, `strictMode`, `enforceX`) with an insecure default means the vendor chose to make the unsafe path operator-controllable. Operator-opt-in misconfig isn't a vendor bug. And an in-code comment that explicitly acknowledges a spec violation is a documented design trade-off, not a defect. All three patterns predict zero payout.\n\nIf all three checks come back clean and the bug-class still looks live, then source-walk. The protocol doesn't replace the audit, it filters out the audits that were never going to pay.\n\n## When this rule does not apply\n\nA few cases are worth flagging. If there's no public issue tracker (proprietary code obtained via mobile-app decompile or an enterprise binary download), there's no public record to consult, so source-walk first. If the maintainer responds to security reports privately via a `security@` intake, a public dup-check is less informative because the meaningful prior-art lives in private threads. If you're inside a 7-day-window sibling-of-CVE-X audit, the fresh CVE may not have hit the issue tracker yet even if it will soon, so source-walk first and treat the dup-check as confirmation. And if the impact is cross-library or cross-version (library v0.0.5 patched but vendor X still ships v0.0.4 in production), the dup-check confirms the library patch but doesn't confirm the deployment, so source-walk plus vendor-deployment-verify is the right shape.\n\n## The meta-pattern\n\nMost bug-hunting frameworks are built around generating hypotheses. The hard part, at least for me, is filtering them before spending ninety minutes confirming a primitive that's been public for a year.\n\nThe pre-mortem paragraph is one of the cheapest filters available. If it names a concretely-testable failure mode, the audit's first action should be to verify or refute that mode. Two findings died on this lesson last week. Hopefully fewer next week.\n\n---\n\n_Methodology surfaced 2026-05-18 during a `cloudflare/workers-oauth-provider` audit. Both findings dead-on-dup at the library tier._\n", "creation_timestamp": "2026-05-17T23:06:40.000000Z"}