-
Notifications
You must be signed in to change notification settings - Fork 3.8k
fix(audit): resolve the client IP from the trusted proxy chain, not the leftmost header #6466
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: staging
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,12 @@ import { stripe } from '@better-auth/stripe' | |
| import { db } from '@sim/db' | ||
| import * as schema from '@sim/db/schema' | ||
| import { createLogger } from '@sim/logger' | ||
| import { | ||
| CLIENT_IP_HEADERS, | ||
| findMalformedTrustedProxies, | ||
| isAllTrustingProxyEntry, | ||
| parseTrustedProxies, | ||
| } from '@sim/security/client-ip' | ||
| import { toError } from '@sim/utils/errors' | ||
| import { type BetterAuthOptions, betterAuth, type User } from 'better-auth' | ||
| import { drizzleAdapter } from 'better-auth/adapters/drizzle' | ||
|
|
@@ -141,15 +147,31 @@ if (validStripeKey) { | |
| } | ||
|
|
||
| /** | ||
| * Reverse-proxy hops trusted for forwarded-IP resolution. When configured, | ||
| * Better Auth walks the x-forwarded-for chain right to left, skips these | ||
| * hops, and records the first untrusted address as the session client IP — | ||
| * preventing header spoofing behind multi-hop proxies. | ||
| * Reverse-proxy hops trusted for forwarded-IP resolution: the chain is walked | ||
| * right to left, these hops are skipped, and the first untrusted address is the | ||
| * client. Parsed with the same helper `resolveClientIp` uses so the session's | ||
| * recorded IP and the one every other caller resolves cannot diverge. | ||
| */ | ||
| const trustedProxies = parseTrustedProxies(env.AUTH_TRUSTED_PROXIES) | ||
|
|
||
| /** | ||
| * Both misconfigurations below leave every request resolving no IP at all, and | ||
| * neither throws — an operator can only discover them if we say so. | ||
| */ | ||
| const trustedProxies = (env.AUTH_TRUSTED_PROXIES ?? '') | ||
| .split(',') | ||
| .map((entry) => entry.trim()) | ||
| .filter(Boolean) | ||
| const malformedTrustedProxies = findMalformedTrustedProxies(trustedProxies) | ||
| if (malformedTrustedProxies.length > 0) { | ||
| logger.error('AUTH_TRUSTED_PROXIES contains entries that are not an IP or CIDR range', { | ||
| malformedTrustedProxies, | ||
| }) | ||
| } | ||
|
|
||
| const allTrustingProxies = trustedProxies.filter(isAllTrustingProxyEntry) | ||
| if (allTrustingProxies.length > 0) { | ||
| logger.error( | ||
| 'AUTH_TRUSTED_PROXIES trusts every hop, so no request will resolve a client IP — scope it to your ingress ranges', | ||
| { allTrustingProxies } | ||
| ) | ||
| } | ||
|
|
||
| export const auth = betterAuth({ | ||
| baseURL: getBaseUrl(), | ||
|
|
@@ -198,6 +220,9 @@ export const auth = betterAuth({ | |
| }, | ||
| advanced: { | ||
| ipAddress: { | ||
| // The same header list and proxy set `resolveClientIp` uses, so the | ||
| // address recorded on a session row is the one every other caller sees. | ||
| ipAddressHeaders: [...CLIENT_IP_HEADERS], | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Session IPv6 diverges from resolverMedium Severity
Additional Locations (1)Reviewed by Cursor Bugbot for commit d6d4180. Configure here. |
||
| ...(trustedProxies.length > 0 ? { trustedProxies } : {}), | ||
| }, | ||
| }, | ||
|
|
||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Turnstile gets unresolved remote IP
Medium Severity
getRateLimitIpKeycan return the sentinelunresolved, and that value is passed straight into Turnstile asremoteIp. The Turnstile helper only skips the oldunknownsentinel, so Cloudflare may receiveremoteip=unresolvedand reject otherwise-valid captchas whenever no trustworthy client IP resolves.Reviewed by Cursor Bugbot for commit d6d4180. Configure here.