Skip to content

🔒 Fix IP Spoofing Vulnerability in Rate Limiter #368

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
+39 −8
Open

🔒 Fix IP Spoofing Vulnerability in Rate Limiter #368

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
20 changes: 14 additions & 6 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,10 +97,10 @@ describe("RateLimiter", () => {
});

describe("getClientIp", () => {
it("returns the right-most x-forwarded-for IP", () => {
it("returns the right-most untrusted x-forwarded-for IP", () => {
const req = new Request("http://localhost", {
headers: {
"x-forwarded-for": "5.6.7.8, 9.10.11.12"
"x-forwarded-for": "5.6.7.8, 9.10.11.12, 10.0.0.1" // 10.0.0.1 is trusted proxy
}
});
expect(getClientIp(req)).toBe("9.10.11.12");
Expand Down Expand Up @@ -147,21 +147,29 @@ describe("getClientIp", () => {
expect(getClientIp(req)).toBe("unknown");
});

it("returns unknown when the right-most x-forwarded-for token is empty", () => {
it("returns the first valid untrusted 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, "
}
});
expect(getClientIp(req)).toBe("unknown");
expect(getClientIp(req)).toBe("5.6.7.8");
});

it("returns unknown when the right-most x-forwarded-for token is invalid", () => {
it("returns the first valid untrusted 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("unknown");
expect(getClientIp(req)).toBe("5.6.7.8");
});
it("returns the left-most IP if all 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("192.168.1.1");
});
});
27 changes: 25 additions & 2 deletions src/lib/rateLimit.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,35 @@ function isValidIp(value: string): boolean {
}
}

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])\.)/;

@greptile-apps greptile-apps Bot Jun 12, 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.

P1 ループバックアドレス範囲の不完全なマッチング

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 IPv6 localhost
const privateIpv6 = /^(::1|fd|fc00::)/;

@greptile-apps greptile-apps Bot Jun 12, 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 IPv6 ULA の fc プレフィックス範囲が不完全

fc00:: というパターンは fc00:: で始まるアドレス（例: fc00::1）にしかマッチせず、fc01::1 や fc12::1 のような fc00::/7 範囲内の他の fc プレフィックスアドレスをカバーできていません。RFC 4193 の ULA 範囲 fc00::/7 は fc と fd の両方のプレフィックスを含むため、fc 単体のパターンに変更すべきです。

Suggested change
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.


return privateIpv4.test(ip) || privateIpv6.test(ip);
}
Comment on lines +69 to +76

@gemini-code-assist gemini-code-assist Bot Jun 12, 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.

security-high high

The isTrustedProxy function has several correctness and security issues:

  1. 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.
  2. Incomplete IPv6 ULA / Link-Local Matching:
    • fc00:: only matches addresses starting with exactly fc00:: (e.g., fc00::1), but fails to match other valid Unique Local Addresses (ULA) like fc01::1 or fc00:1::1 (since ULA is fc00::/7, which covers fc00:: to fdff::).
    • Link-local IPv6 addresses (fe80::/10) are not matched at all.
  3. Incomplete IPv4 Loopback / Link-Local Matching:
    • Only 127.0.0.1 is matched, but the entire 127.0.0.0/8 block is reserved for loopback (e.g., 127.0.0.2).
    • Link-local IPv4 addresses (169.254.0.0/16) are not matched.

We can fix these issues by updating the regexes to be case-insensitive and cover the full ranges.

Suggested change
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);
}


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

const proxyObservedIp = forwardedFor.split(",").at(-1)?.trim();
if (proxyObservedIp && isValidIp(proxyObservedIp)) return proxyObservedIp;
const ips = forwardedFor.split(",").map(ip => ip.trim());

// Iterate from right to left to find the first non-trusted IP
// For this example, we assume we want to skip internal/private IPs (trusted proxies)
// and find the true client IP.
Comment on lines +85 to +86

@greptile-apps greptile-apps Bot Jun 12, 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 "For this example" は本番コードに残すべきでないコメント

コメント内の「For this example」という表現は、プロトタイプや例示コードの名残のような印象を与えます。本番コードにはより正確な意図を示す表現を使用すべきです。

Suggested change
// 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!

for (let i = ips.length - 1; i >= 0; i--) {
const ip = ips[i];
if (ip && isValidIp(ip) && !isTrustedProxy(ip)) {
return ip;
}
}

// Fallback: If all are trusted (which is unlikely for a client request, but possible if they spoof),
// or if we couldn't find a valid non-trusted one, return the left-most valid IP.
const firstIp = ips[0];
if (firstIp && isValidIp(firstIp)) return firstIp;

return "unknown";
}
Loading