🔒 Fix IP Spoofing Vulnerability in Rate Limiter#368
Conversation
Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
|
👋 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 New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
Warning Review limit reached
More reviews will be available in 46 minutes. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the getClientIp function to identify the true client IP by skipping trusted proxies (private and internal IPs) when parsing the x-forwarded-for header, iterating from right to left. It also adds a helper function isTrustedProxy and updates the test suite. The review feedback points out critical security and correctness issues in the isTrustedProxy regex matching, such as case-sensitivity for IPv6 addresses and incomplete coverage for loopback and link-local ranges, and provides a robust code suggestion to address these vulnerabilities.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| function isTrustedProxy(ip: string): boolean { | ||
| // Matches standard private IPv4 ranges (RFC 1918) and localhost | ||
| const privateIpv4 = /^(127\.0\.0\.1|10\.|192\.168\.|172\.(1[6-9]|2[0-9]|3[0-1])\.)/; | ||
| // Matches IPv6 localhost | ||
| const privateIpv6 = /^(::1|fd|fc00::)/; | ||
|
|
||
| return privateIpv4.test(ip) || privateIpv6.test(ip); | ||
| } |
There was a problem hiding this comment.
The isTrustedProxy function has several correctness and security issues:
- Case Sensitivity: IPv6 addresses can be uppercase (e.g.,
FD00::1). The regex/^(::1|fd|fc00::)/is case-sensitive and will fail to match uppercase IPv6 addresses, potentially allowing rate-limiting bypasses or causing Denial of Service (DoS) if trusted proxies are misclassified as untrusted client IPs. - Incomplete IPv6 ULA / Link-Local Matching:
fc00::only matches addresses starting with exactlyfc00::(e.g.,fc00::1), but fails to match other valid Unique Local Addresses (ULA) likefc01::1orfc00:1::1(since ULA isfc00::/7, which coversfc00::tofdff::).- Link-local IPv6 addresses (
fe80::/10) are not matched at all.
- Incomplete IPv4 Loopback / Link-Local Matching:
- Only
127.0.0.1is matched, but the entire127.0.0.0/8block is reserved for loopback (e.g.,127.0.0.2). - Link-local IPv4 addresses (
169.254.0.0/16) are not matched.
- Only
We can fix these issues by updating the regexes to be case-insensitive and cover the full ranges.
| function isTrustedProxy(ip: string): boolean { | |
| // Matches standard private IPv4 ranges (RFC 1918) and localhost | |
| const privateIpv4 = /^(127\.0\.0\.1|10\.|192\.168\.|172\.(1[6-9]|2[0-9]|3[0-1])\.)/; | |
| // Matches IPv6 localhost | |
| const privateIpv6 = /^(::1|fd|fc00::)/; | |
| return privateIpv4.test(ip) || privateIpv6.test(ip); | |
| } | |
| function isTrustedProxy(ip: string): boolean { | |
| // Matches standard private IPv4 ranges (RFC 1918), loopback (127.0.0.0/8), and link-local (169.254.0.0/16) | |
| const privateIpv4 = /^(127\.|10\.|192\.168\.|169\.254\.|172\.(1[6-9]|2[0-9]|3[0-1])\.)/; | |
| // Matches IPv6 loopback (::1), Unique Local Addresses (fc00::/7), and Link-Local Addresses (fe80::/10) | |
| const privateIpv6 = /^(::1|f[cd]|fe[89ab])/i; | |
| return privateIpv4.test(ip) || privateIpv6.test(ip); | |
| } |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
|
||
| function isTrustedProxy(ip: string): boolean { | ||
| // Matches standard private IPv4 ranges (RFC 1918) and localhost | ||
| const privateIpv4 = /^(127\.0\.0\.1|10\.|192\.168\.|172\.(1[6-9]|2[0-9]|3[0-1])\.)/; |
There was a problem hiding this comment.
127\.0\.0\.1 は単一のアドレスのみにマッチしますが、ループバック範囲 127.0.0.0/8 全体（127.0.0.1 ～ 127.255.255.255）をカバーしていません。127.0.0.2 など 127.0.0.1 以外のループバックアドレスをプロキシとして使用している環境では、そのアドレスが「信頼済みプロキシ」とみなされず、誤ってクライアント IP として返されます。結果として、そのプロキシを経由するすべてのリクエストが同一のレートリミットバケットに集約され、正当なユーザーが不当にブロックされる可能性があります。
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/rateLimit.ts
Line: 71
Comment:
**ループバックアドレス範囲の不完全なマッチング**
`127\.0\.0\.1` は単一のアドレスのみにマッチしますが、ループバック範囲 `127.0.0.0/8` 全体（`127.0.0.1` ～ `127.255.255.255`）をカバーしていません。`127.0.0.2` など `127.0.0.1` 以外のループバックアドレスをプロキシとして使用している環境では、そのアドレスが「信頼済みプロキシ」とみなされず、誤ってクライアント IP として返されます。結果として、そのプロキシを経由するすべてのリクエストが同一のレートリミットバケットに集約され、正当なユーザーが不当にブロックされる可能性があります。
How can I resolve this? If you propose a fix, please make it concise.| // Matches standard private IPv4 ranges (RFC 1918) and localhost | ||
| const privateIpv4 = /^(127\.0\.0\.1|10\.|192\.168\.|172\.(1[6-9]|2[0-9]|3[0-1])\.)/; | ||
| // Matches IPv6 localhost | ||
| const privateIpv6 = /^(::1|fd|fc00::)/; |
There was a problem hiding this comment.
fc00:: というパターンは fc00:: で始まるアドレス（例: fc00::1）にしかマッチせず、fc01::1 や fc12::1 のような fc00::/7 範囲内の他の fc プレフィックスアドレスをカバーできていません。RFC 4193 の ULA 範囲 fc00::/7 は fc と fd の両方のプレフィックスを含むため、fc 単体のパターンに変更すべきです。
| const privateIpv6 = /^(::1|fd|fc00::)/; | |
| const privateIpv6 = /^(::1|fc|fd)/; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/rateLimit.ts
Line: 73
Comment:
**IPv6 ULA の fc プレフィックス範囲が不完全**
`fc00::` というパターンは `fc00::` で始まるアドレス（例: `fc00::1`）にしかマッチせず、`fc01::1` や `fc12::1` のような `fc00::/7` 範囲内の他の `fc` プレフィックスアドレスをカバーできていません。RFC 4193 の ULA 範囲 `fc00::/7` は `fc` と `fd` の両方のプレフィックスを含むため、`fc` 単体のパターンに変更すべきです。
```suggestion
const privateIpv6 = /^(::1|fc|fd)/;
```
How can I resolve this? If you propose a fix, please make it concise.| // For this example, we assume we want to skip internal/private IPs (trusted proxies) | ||
| // and find the true client IP. |
There was a problem hiding this comment.
コメント内の「For this example」という表現は、プロトタイプや例示コードの名残のような印象を与えます。本番コードにはより正確な意図を示す表現を使用すべきです。
| // For this example, we assume we want to skip internal/private IPs (trusted proxies) | |
| // and find the true client IP. | |
| // 右端から左に向かって走査し、最初の非プライベート（非信頼）IPを真のクライアントIPとして返す。 |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/rateLimit.ts
Line: 85-86
Comment:
**"For this example" は本番コードに残すべきでないコメント**
コメント内の「For this example」という表現は、プロトタイプや例示コードの名残のような印象を与えます。本番コードにはより正確な意図を示す表現を使用すべきです。
```suggestion
// 右端から左に向かって走査し、最初の非プライベート（非信頼）IPを真のクライアントIPとして返す。
```
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
1 participant
🎯 What: The vulnerability fixed
The
getClientIpfunction in the rate limiter was vulnerable to IP spoofing. It improperly extracted the right-most IP address from theX-Forwarded-Forheader.An attacker could bypass rate limits entirely by manually providing a spoofed
X-Forwarded-Forheader. In a typical reverse-proxy setup, the proxy appends the true client IP to the end of the header. By taking the right-most IP, the application was susceptible to extracting an internal proxy IP or trusting the left-most spoofed IP in certain fallback scenarios, rendering the rate limiter ineffective and opening the application up to DoS attacks.🛡️ Solution: How the fix addresses the vulnerability
The extraction logic was updated to implement standard proxy-aware parsing:
X-Forwarded-Forcomma-separated list from right to left.isTrustedProxyhelper to skip over any known private/internal IPs (RFC 1918 and localhost ranges)."unknown".Test suites were also updated to validate this new secure behavior.
PR created automatically by Jules for task 14757583944471206086 started by @is0692vs
Greptile Summary
このPRは、レートリミッターの
getClientIp関数における IP スプーフィング脆弱性を修正します。X-Forwarded-Forヘッダーを右から左に走査し、RFC 1918 のプライベート範囲をスキップするisTrustedProxyヘルパーを新たに導入しています。isTrustedProxyを追加して既知の内部/プライベート IP を識別し、X-Forwarded-Forを右端から走査して最初の非信頼 IP を真のクライアント IP として返すようgetClientIpを修正。"unknown"を返すロジックを追加。Confidence Score: 3/5
ループバック範囲の不完全なマッチングにより、一部の環境では正しいクライアント IP が特定されない可能性があります。
isTrustedProxyのループバックチェックが127.0.0.1のみを対象としており127.0.0.0/8全体をカバーしていないため、127.0.0.2などのアドレスをプロキシとして使用する環境では誤ったクライアント IP がレートリミットのキーとして使われてしまいます。意図としては正しい方向性の修正ですが、この抜け漏れが本番環境で誤動作を招く可能性があります。src/lib/rateLimit.ts の
isTrustedProxy関数を重点的に確認してください。Important Files Changed
isTrustedProxyヘルパーと右から左への XFF 走査ロジックを追加。ただしループバック範囲が127.0.0.1のみに限定され127.0.0.0/8全体をカバーしていない点、および IPv6 ULA のfcプレフィックス検出が不完全な点に問題あり。127.0.0.0/8全体のカバレッジテストは含まれていない。Sequence Diagram
sequenceDiagram participant Client as クライアント (203.0.113.1) participant Proxy as プロキシ (10.0.0.1) participant App as アプリケーション participant getClientIp as getClientIp() participant isTrustedProxy as isTrustedProxy() Client->>Proxy: リクエスト送信 Note over Proxy: XFF に接続元 IP を追記 Proxy->>App: X-Forwarded-For: 203.0.113.1, 10.0.0.1 App->>getClientIp: request を渡す getClientIp->>getClientIp: IPs を右から走査 getClientIp->>isTrustedProxy: "10.0.0.1" をチェック isTrustedProxy-->>getClientIp: true (プライベート IP) getClientIp->>isTrustedProxy: "203.0.113.1" をチェック isTrustedProxy-->>getClientIp: false (パブリック IP) getClientIp-->>App: "203.0.113.1" を返す App->>App: RateLimiter.check("203.0.113.1")Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "fix(security): resolve IP spoofing vulne..." | Re-trigger Greptile