Skip to content

🔒 Fix Rate Limit Bypass via X-Forwarded-For Spoofing #524

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.

Sign up for GitHub

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

Open
is0692vs wants to merge 1 commit into
base: main
Choose a base branch
from
Open

🔒 Fix Rate Limit Bypass via X-Forwarded-For Spoofing #524

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 8 additions & 33 deletions src/lib/__tests__/rateLimit.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,13 +97,14 @@ describe("RateLimiter", () => {
});

describe("getClientIp", () => {
it("returns the right-most untrusted x-forwarded-for IP", () => {
it("returns the right-most valid x-forwarded-for IP without skipping private IPs", () => {
const req = new Request("http://localhost", {
headers: {
"x-forwarded-for": "5.6.7.8, 9.10.11.12, 10.0.0.1" // 10.0.0.1 is trusted proxy
"x-forwarded-for": "5.6.7.8, 9.10.11.12, 10.0.0.1"
}
});
expect(getClientIp(req)).toBe("9.10.11.12");
// 10.0.0.1 should not be skipped anymore, it is the right-most valid IP.
expect(getClientIp(req)).toBe("10.0.0.1");
});

it("trims whitespace from the selected x-forwarded-for IP", () => {
Expand Down Expand Up @@ -147,7 +148,7 @@ describe("getClientIp", () => {
expect(getClientIp(req)).toBe("unknown");
});

it("returns the first valid untrusted IP when the right-most x-forwarded-for token is empty", () => {
it("returns the first valid IP when the right-most x-forwarded-for token is empty", () => {
const req = new Request("http://localhost", {
headers: {
"x-forwarded-for": "5.6.7.8, "
Expand All @@ -156,47 +157,21 @@ describe("getClientIp", () => {
expect(getClientIp(req)).toBe("5.6.7.8");
});

it("returns the first valid untrusted IP when the right-most x-forwarded-for token is invalid", () => {
it("returns the first valid IP when the right-most x-forwarded-for token is invalid", () => {
const req = new Request("http://localhost", {
headers: {
"x-forwarded-for": "5.6.7.8, not-an-ip"
}
});
expect(getClientIp(req)).toBe("5.6.7.8");
});
it.each([
["IPv4 loopback", "203.0.113.10, 127.0.0.2"],
["IPv4 link-local", "203.0.113.10, 169.254.10.20"],
["IPv4-mapped IPv6 private IPv4", "203.0.113.10, ::ffff:10.0.0.1"],
["IPv4-mapped IPv6 loopback", "203.0.113.10, ::ffff:127.0.0.2"],
["IPv4-mapped IPv6 link-local", "203.0.113.10, ::ffff:169.254.10.20"],
["IPv6 ULA fc prefix", "203.0.113.10, fc12::1"],
["IPv6 ULA uppercase fd prefix", "203.0.113.10, FD00::1"],
["IPv6 link-local", "203.0.113.10, fe80::1"],
])("skips trusted proxy range: %s", (_, forwardedFor) => {
const req = new Request("http://localhost", {
headers: {
"x-forwarded-for": forwardedFor
}
});
expect(getClientIp(req)).toBe("203.0.113.10");
});

it("returns unknown if all x-forwarded-for IPs are trusted proxies", () => {
const req = new Request("http://localhost", {
headers: {
"x-forwarded-for": "192.168.1.1, 10.0.0.1"
}
});
expect(getClientIp(req)).toBe("unknown");
});

it("returns unknown if a spoofed private chain contains no public client IP", () => {
it("does not skip spoofed private chains", () => {
const req = new Request("http://localhost", {
headers: {
"x-forwarded-for": "127.0.0.2, 169.254.10.20, fd00::1"
}
});
expect(getClientIp(req)).toBe("unknown");
expect(getClientIp(req)).toBe("fd00::1");
});
});
21 changes: 4 additions & 17 deletions src/lib/rateLimit.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,31 +66,18 @@ function isValidIp(value: string): boolean {
}
}

function isTrustedProxy(ip: string): boolean {
// Matches private IPv4 ranges, loopback, and link-local addresses.
const privateIpv4 = /^(127\.|10\.|192\.168\.|169\.254\.|172\.(1[6-9]|2[0-9]|3[0-1])\.)/;
// Matches IPv4-mapped IPv6 versions of the same private IPv4 ranges.
const privateIpv4MappedIpv6 = /^::ffff:(127\.|10\.|192\.168\.|169\.254\.|172\.(1[6-9]|2[0-9]|3[0-1])\.)/i;
// Matches IPv6 loopback, ULA (fc00::/7), and link-local (fe80::/10).
const privateIpv6 = /^(::1|f[cd]|fe[89ab])/i;

return (
privateIpv4.test(ip) ||
privateIpv4MappedIpv6.test(ip) ||
privateIpv6.test(ip)
);
}

export function getClientIp(request: Request): string {
const forwardedFor = request.headers.get("x-forwarded-for");
if (!forwardedFor) return "unknown";

const ips = forwardedFor.split(",").map(ip => ip.trim());

// Iterate from right to left and return the first non-private IP as the client IP.
// Iterate from right to left and return the first valid IP.
// We do not skip private IPs because doing so without a known proxy topology
// allows attackers to bypass rate limiting by appending a spoofed private IP.
for (let i = ips.length - 1; i >= 0; i--) {
const ip = ips[i];
if (ip && isValidIp(ip) && !isTrustedProxy(ip)) {
if (ip && isValidIp(ip)) {
return ip;
Comment on lines +80 to 81

@greptile-apps greptile-apps Bot Aug 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 プライベートプロキシで共有キー化

プロキシが自身のプライベート IP を X-Forwarded-For の右端に追加する環境では、その IP が全クライアント共通のレート制限キーになります。そのため、1 クライアントが上限を消費すると、同じプロキシ配下の他クライアントにも 429 が返されます。

Knowledge Base Used: GitHub Client

Prompt To Fix With AI 
This is a comment left during a code review.
Path: src/lib/rateLimit.ts
Line: 80-81

Comment:
**プライベートプロキシで共有キー化**

プロキシが自身のプライベート IP を `X-Forwarded-For` の右端に追加する環境では、その IP が全クライアント共通のレート制限キーになります。そのため、1 クライアントが上限を消費すると、同じプロキシ配下の他クライアントにも 429 が返されます。

**Knowledge Base Used:** [GitHub Client](https://app.greptile.com/hiroki-org/-/custom-context/knowledge-base/hiroki-org/github-user-summary/-/docs/github-client.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Comment on lines +75 to 81

@qodo-code-review qodo-code-review Bot Aug 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

1. Proxy ip rate limiting 🐞 Bug ≡ Correctness

getClientIp now returns the right-most syntactically valid X-Forwarded-For token even if it is a
private proxy address, and the API routes use that value directly as the rate-limit key. In
deployments where the right-most XFF entry is a shared internal proxy hop, this can group many users
into one bucket and cause widespread false 429s.
Agent Prompt 
## Issue description
`getClientIp()` now always returns the right-most valid `x-forwarded-for` token, including private IPs. When the right-most token represents a shared internal proxy hop, using it as the rate-limit key collapses many users into a single bucket and causes false throttling.

## Issue Context
- `/api/card` and `/api/og` both call `rateLimiter.check(getClientIp(request))`.
- The updated tests explicitly lock in returning a private right-most token (e.g., `10.0.0.1`).
- The security fix is valid for spoofing resistance, but the new behavior should be configurable so environments with known proxy topology can still rate-limit by the true client identity.

## Fix Focus Areas
- Add an explicit, opt-in trust model (e.g., trusted proxy hop count and/or trusted proxy CIDRs via env var) and derive the client IP accordingly.
- Keep the secure default behavior when no trust configuration is provided.
- Update/extend tests to cover both the secure default and the configured trusted-proxy mode.

### Code references
- src/lib/rateLimit.ts[69-86]
- src/app/api/card/[username]/route.ts[23-25]
- src/app/api/og/[username]/route.tsx[21-23]
- src/lib/__tests__/rateLimit.test.ts[99-108]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

}
}
Expand Down
Loading