Skip to content

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

Open
is0692vs wants to merge 1 commit into
mainfrom
security/fix-rate-limit-ip-spoofing-7941677267623678207
Open

🔒 Fix Rate Limit Bypass via X-Forwarded-For Spoofing#524
is0692vs wants to merge 1 commit into
mainfrom
security/fix-rate-limit-ip-spoofing-7941677267623678207

Conversation

@is0692vs

@is0692vs is0692vs commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

🎯 What: Fixed an IP spoofing vulnerability in the rate limiter's X-Forwarded-For parsing logic that allowed attackers to bypass limits.
⚠️ Risk: The previous logic attempted to automatically skip "trusted" (private) IPs when reading the header from right to left. Attackers could exploit this by sending an X-Forwarded-For header containing a target public IP followed by a spoofed private IP (e.g., X-Forwarded-For: <spoofed_ip>, 127.0.0.1). The parser would skip the private IP and incorrectly attribute the request to the attacker's chosen public IP, completely bypassing rate limits.
🛡️ Solution: Removed the unsafe isTrustedProxy skipping logic. When the proxy topology is unknown, blindly trusting private IPs is a security anti-pattern. The safest default behavior is to use the right-most valid IP in the chain without skipping. This ensures that even if an attacker attempts to spoof the header, they cannot easily inject a proxy layer to bypass the check. Associated tests were updated to reflect this secure behavior.


PR created automatically by Jules for task 7941677267623678207 started by @is0692vs

Greptile Summary

この PR は、X-Forwarded-For の右端からプライベート IP を除外していた処理を削除し、右端の有効な IP をそのままレート制限キーに採用します。

  • isTrustedProxy とプライベート/ループバック/リンクローカル判定を削除
  • プライベート IP を選択する新しい仕様に合わせて単体テストを更新
  • 未知のプロキシ構成における IP 偽装経路を縮小する一方、プライベート IP を追加する実プロキシ配下では共有バケット化する可能性がある

Confidence Score: 4/5

PR はマージ可能ですが、プライベート IP を追加するリバースプロキシ配下でレート制限が共有バケット化する非ブロッキングの構成上の懸念があります。

変更は説明された偽装経路を閉じていますが、getClientIp の結果を直接キーにする既存 API では、右端のプライベートプロキシ IP が全クライアント共通の識別子になります。

Files Needing Attention: src/lib/rateLimit.ts

Important Files Changed

Filename Overview
src/lib/rateLimit.ts プライベート IP の除外を廃止して偽装経路を修正する変更だが、プライベート IP を追加する実プロキシでは全利用者が同じレート制限キーになる。
src/lib/tests/rateLimit.test.ts プライベート IP を含む右端の有効なトークンを選択する新仕様に合わせてテストを更新している。

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  C1[クライアント A] --> P[プライベート IP のリバースプロキシ]
  C2[クライアント B] --> P
  P -->|X-Forwarded-For の右端にプロキシ IP| G[getClientIp]
  G -->|同一のプロキシ IP| R[共有レート制限バケット]
  R -->|上限到達| E[両クライアントへ 429]
Loading
Prompt To Fix All With AI
### Issue 1
src/lib/rateLimit.ts:80-81
**プライベートプロキシで共有キー化**

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

---

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

Reviews (1): Last reviewed commit: "Fix rate limit IP spoofing vulnerability..." | Re-trigger Greptile

Greptile also left 1 inline comment on this PR.

Context used:

Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
@google-labs-jules

Copy link
Copy Markdown
Contributor

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@vercel

vercel Bot commented Aug 1, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
github-user-summary Ignored Ignored Aug 1, 2026 4:45am

@gemini-code-assist

Copy link
Copy Markdown

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@is0692vs, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 39 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 654270e7-ee4b-4072-bd08-118f3970597b

📥 Commits

Reviewing files that changed from the base of the PR and between eb95c48 and 9573b71.

📒 Files selected for processing (2)
  • src/lib/__tests__/rateLimit.test.ts
  • src/lib/rateLimit.ts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@dosubot

dosubot Bot commented Aug 1, 2026

Copy link
Copy Markdown

📄 Knowledge review

Dosu skipped reviewing this PR because your organization has used its 200 included credits for the month. Your usage will reset on 2026-09-01. To have Dosu review this PR before then, ask your organization admin to upgrade to a pro account.


Leave Feedback Ask Dosu about github-user-summary Add Dosu to your team

@dosubot dosubot Bot added the bug Something isn't working label Aug 1, 2026
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Fix rate-limit bypass by hardening X-Forwarded-For IP selection

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

Grey Divider

AI Description

• Stop skipping private IPs when parsing X-Forwarded-For to prevent spoofing bypass.
• Select the right-most valid IP token as the client IP by default.
• Update tests to reflect the new, safer header parsing behavior.
Diagram

graph TD
  A(["Incoming Request"]) --> B["getClientIp()"] --> C["Split XFF tokens"] --> D{"Token is valid IP?"}
  D -->|"yes (right-most)"| E["Return selected IP"] --> F(["RateLimiter keying"])
  D -->|"no"| G["Scan left for first valid"] --> E

  subgraph Legend
    direction LR
    _req(["Request"]) ~~~ _fn["Function"] ~~~ _dec{"Decision"}
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Configure explicit trusted proxies / hop count
  • ➕ Supports common deployments (LB/CDN + internal proxies) while remaining secure
  • ➕ Allows skipping known proxy IPs safely instead of relying on private-range heuristics
  • ➖ Requires configuration management and correct ops setup
  • ➖ Misconfiguration can reintroduce bypass or lock out legitimate traffic
2. Use standard Forwarded header parsing (RFC 7239) with policy
  • ➕ More structured than X-Forwarded-For and less ambiguous
  • ➕ Can enforce a clear trust policy per proxy
  • ➖ Not universally provided by all proxies; may require infrastructure changes
  • ➖ Still needs a trust model (which proxies are authoritative)

Recommendation: The PR’s approach (take the right-most valid IP without skipping private ranges) is the safest default when proxy topology is unknown, and directly closes the bypass vector. If the product requires deriving the true end-user IP behind known infrastructure, add an opt-in configuration for trusted proxy IPs/hop-count and keep the current behavior as the secure default.

Files changed (2) +12 / -50

Bug fix (1) +4 / -17
rateLimit.tsHarden getClientIp by removing trusted-proxy skipping +4/-17

Harden getClientIp by removing trusted-proxy skipping

• Deletes the isTrustedProxy() private-range heuristics and updates getClientIp() to return the right-most valid IP in X-Forwarded-For. Adds rationale in comments explaining why skipping private IPs is unsafe without a known proxy topology.

src/lib/rateLimit.ts

Tests (1) +8 / -33
rateLimit.test.tsUpdate getClientIp tests to match secure XFF parsing behavior +8/-33

Update getClientIp tests to match secure XFF parsing behavior

• Adjusts expectations to ensure private IPs are no longer skipped when selecting the client IP. Removes parameterized tests that asserted trusted-proxy skipping and replaces them with a test verifying spoofed private chains are not treated specially.

src/lib/tests/rateLimit.test.ts

@codecov

codecov Bot commented Aug 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Comment thread src/lib/rateLimit.ts
Comment on lines +80 to 81
if (ip && isValidIp(ip)) {
return ip;

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.

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 30 rules

Grey Divider


Remediation recommended

1. Proxy IP rate limiting 🐞 Bug ≡ Correctness
Description
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.
Code

src/lib/rateLimit.ts[R75-81]

+    // 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;
Evidence
The PR changes getClientIp() to return the first valid token scanning right-to-left without
skipping private ranges, and the application uses that value directly as the rate-limit key. The
updated test demonstrates that a private 10.0.0.1 right-most token is now considered the client
IP, which can become a shared bucket if it represents a proxy hop.

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]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## 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


Grey Divider

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

Comment thread src/lib/rateLimit.ts
Comment on lines +75 to 81
// 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;

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working size/M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant