github-starred/better-auth
2
0
Fork 0
mirror of https://github.com/better-auth/better-auth.git synced 2026-05-22 14:21:55 -05:00
Code Issues 2.5k Packages Projects Releases 112 Wiki Activity

[GH-ISSUE #8887] Security & input validation findings in state handling, URL validation, and password utils (PatchPilots audit) #28545

New Issue
Closed
opened 2026-04-17 19:59:51 -05:00 by GiteaMirror · 2 comments
GiteaMirror commented 2026-04-17 19:59:51 -05:00
Owner
Copy Link

Originally created by @alavesa on GitHub (Apr 1, 2026).
Original GitHub issue: https://github.com/better-auth/better-auth/issues/8887

Security & input validation findings from PatchPilots audit

I ran PatchPilots (an AI code review tool) against packages/better-auth/src — 20 files across state handling, URL validation, and password utilities. Cross-referenced against existing open issues to avoid duplicates.

Medium severity

1. OAuth state not verified in cookie-based strategy (state.ts)

In parseGenericState when storeStateStrategy === 'cookie', the state value from the OAuth callback URL is never compared against the originally generated state value. The function only checks that the oauth_state cookie exists and can be decrypted — but doesn't verify that the returned state matches what was sent. The database strategy does perform this comparison.

Impact: Potential CSRF in OAuth flows using the cookie strategy.

Note: Existing issues (#5243, #7352, #6847, #5871) report state mismatch bugs — this is a different concern: the verification step itself is missing.

2. ReDoS in proxy host header validation regex (utils/url.ts)

The hostnameRegex in validateProxyHeader uses nested quantifiers: ([a-zA-Z0-9-]{0,61}[a-zA-Z0-9])? repeated via the outer group. This can trigger catastrophic backtracking on crafted x-forwarded-host header values (e.g., a long string ending in -).

Impact: An attacker sending malicious x-forwarded-host headers could cause CPU exhaustion.

3. No maximum password length check before hashing (utils/password.ts)

Neither validatePassword nor checkPassword enforces a maximum length before passing the password to the hashing function. Bcrypt is intentionally slow — feeding it a multi-megabyte string can monopolize CPU.

Impact: DoS via extremely long password submissions.

Note: #779 discusses password complexity rules but not maximum length as a DoS vector.

4. Account enumeration via distinct error codes (utils/password.ts)

checkPassword throws CREDENTIAL_ACCOUNT_NOT_FOUND when no account exists and INVALID_PASSWORD when the password is wrong. An attacker can distinguish between valid and invalid accounts by observing the error code.

Impact: Account enumeration on the sign-in flow.

Note: Related to existing enumeration work in #5017 (OTP email) and #8096 (/change-email), but this is a separate code path in the credential sign-in flow.

Low severity

5. IPv4 validation allows invalid octets (utils/url.ts)

The IPv4 regex /^(\d{1,3}\.){3}\d{1,3}(:[0-9]{1,5})?$/ accepts values like 999.999.999.999 since it only constrains digit count, not the numeric range (0–255).

6. Port validation allows ports above 65535 (utils/url.ts)

The port regex (:[0-9]{1,5})? permits five-digit ports like 99999 or 80000, which are not valid TCP ports (valid: 0–65535).

Tool: PatchPilots v0.7.3 — 8 AI agents for automated code review
Audit scope: 20 files, reviewer + security agents
Cost: ~$1.78 in Claude API tokens

Originally created by @alavesa on GitHub (Apr 1, 2026). Original GitHub issue: https://github.com/better-auth/better-auth/issues/8887 ## Security & input validation findings from PatchPilots audit I ran [PatchPilots](https://github.com/alavesa/patchpilots) (an AI code review tool) against `packages/better-auth/src` — 20 files across state handling, URL validation, and password utilities. Cross-referenced against existing open issues to avoid duplicates. ### Medium severity #### 1. OAuth state not verified in cookie-based strategy (`state.ts`) In `parseGenericState` when `storeStateStrategy === 'cookie'`, the `state` value from the OAuth callback URL is never compared against the originally generated state value. The function only checks that the `oauth_state` cookie exists and can be decrypted — but doesn't verify that the returned state matches what was sent. The database strategy does perform this comparison. **Impact:** Potential CSRF in OAuth flows using the cookie strategy. *Note: Existing issues (#5243, #7352, #6847, #5871) report state mismatch bugs — this is a different concern: the verification step itself is missing.* #### 2. ReDoS in proxy host header validation regex (`utils/url.ts`) The `hostnameRegex` in `validateProxyHeader` uses nested quantifiers: `([a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?` repeated via the outer group. This can trigger catastrophic backtracking on crafted `x-forwarded-host` header values (e.g., a long string ending in `-`). **Impact:** An attacker sending malicious `x-forwarded-host` headers could cause CPU exhaustion. #### 3. No maximum password length check before hashing (`utils/password.ts`) Neither `validatePassword` nor `checkPassword` enforces a maximum length before passing the password to the hashing function. Bcrypt is intentionally slow — feeding it a multi-megabyte string can monopolize CPU. **Impact:** DoS via extremely long password submissions. *Note: #779 discusses password complexity rules but not maximum length as a DoS vector.* #### 4. Account enumeration via distinct error codes (`utils/password.ts`) `checkPassword` throws `CREDENTIAL_ACCOUNT_NOT_FOUND` when no account exists and `INVALID_PASSWORD` when the password is wrong. An attacker can distinguish between valid and invalid accounts by observing the error code. **Impact:** Account enumeration on the sign-in flow. *Note: Related to existing enumeration work in #5017 (OTP email) and #8096 (/change-email), but this is a separate code path in the credential sign-in flow.* ### Low severity #### 5. IPv4 validation allows invalid octets (`utils/url.ts`) The IPv4 regex `/^(\d{1,3}\.){3}\d{1,3}(:[0-9]{1,5})?$/` accepts values like `999.999.999.999` since it only constrains digit count, not the numeric range (0–255). #### 6. Port validation allows ports above 65535 (`utils/url.ts`) The port regex `(:[0-9]{1,5})?` permits five-digit ports like `99999` or `80000`, which are not valid TCP ports (valid: 0–65535). --- **Tool:** [PatchPilots v0.7.3](https://www.npmjs.com/package/patchpilots) — 8 AI agents for automated code review **Audit scope:** 20 files, reviewer + security agents **Cost:** ~$1.78 in Claude API tokens
GiteaMirror added the securitybug labels 2026-04-17 19:59:51 -05:00
GiteaMirror commented 2026-04-17 19:59:55 -05:00
Author
Owner
Copy Link

@bytaesu commented on GitHub (Apr 1, 2026):

Closing this issue. It bundles too many tasks into one, and it doesn’t seem possible to discuss it with the author. Please open a separate issue if you have further opinions.

<!-- gh-comment-id:4172360097 --> @bytaesu commented on GitHub (Apr 1, 2026): Closing this issue. It bundles too many tasks into one, and it doesn’t seem possible to discuss it with the author. Please open a separate issue if you have further opinions.
GiteaMirror commented 2026-04-17 19:59:55 -05:00
Author
Owner
Copy Link

@alavesa commented on GitHub (Apr 1, 2026):

I've split the findings into individual issues for easier tracking. Happy to discuss any of them.

<!-- gh-comment-id:4172723085 --> @alavesa commented on GitHub (Apr 1, 2026): I've split the findings into individual issues for easier tracking. Happy to discuss any of them.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
adapter
astro
awaiting external contributor
blocked
breaking
breaking change
bug
c-devops
core
credentials
database
dependencies
devops
devtools
docs
documentation
duplicate
elysia
enhancement
enterprise
expo
express
fastify
good first issue
help wanted
hono
identity
infra
integration
invalid
javascript
locked
maintenance
need-more-information
needs: info
needs: repro
nextjs
nuxt
oauth
organization
P0
payments
perf
platform
plugin
pull-request
Mirrored from GitHub Pull Request
 question
ready
regression
remix
security
social-provider
solid
stale
svelte
tanstack-start
tracking
version-bump
vue
wontfix
No Label bug security
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
GiteaMirror ninjasurge
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/better-auth#28545
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?