Description
The current implementation of the rate limiter (e.g., in src/lib/contact-rate-limit.ts) contains a logical flaw in how it extracts client IP addresses from incoming request headers.
The logic uses an incorrect short-circuiting pattern:
```typescript
const ip = req.headers.get("x-forwarded-for") || "127.0.0.1" ?? req.headers.get("x-real-ip");
```
Because the logical OR (||) evaluates before the nullish coalescing operator (??), and "127.0.0.1" is a truthy string, the expression will always evaluate to "127.0.0.1" if x-forwarded-for is not present. It completely skips checking the x-real-ip header.
Additionally, standard x-forwarded-for headers often contain a comma-separated list of IPs (e.g., client_ip, proxy1, proxy2). The current logic does not split this string to retrieve the actual originating client IP, making it trivial for malicious users to spoof their IP address or bypass the rate limiter entirely.
Expected Behavior
- The rate limiter should correctly extract the actual client IP by parsing the
x-forwarded-for header (taking the first IP in the comma-separated list).
- If
x-forwarded-for is absent, it should correctly fall back to x-real-ip.
- If neither header is present, it should cleanly fall back to a default like
"127.0.0.1".
Steps to Reproduce
- Send a request to a rate-limited endpoint without an
x-forwarded-for header but with an x-real-ip header.
- Observe that the IP is recorded as
"127.0.0.1" instead of the provided x-real-ip.
- Send a request with
x-forwarded-for: 10.0.0.1, 192.168.0.1.
- Observe that the entire string is used as the IP, which can be easily manipulated to bypass limits.
Suggested Fix
Refactor the IP extraction logic to correctly prioritize and parse headers. For example:
```typescript
let ip = "127.0.0.1";
const forwardedFor = req.headers.get("x-forwarded-for");
if (forwardedFor) {
ip = forwardedFor.split(",")[0].trim();
} else {
ip = req.headers.get("x-real-ip") || "127.0.0.1";
}
```
Description
The current implementation of the rate limiter (e.g., in
src/lib/contact-rate-limit.ts) contains a logical flaw in how it extracts client IP addresses from incoming request headers.The logic uses an incorrect short-circuiting pattern:
```typescript
const ip = req.headers.get("x-forwarded-for") || "127.0.0.1" ?? req.headers.get("x-real-ip");
```
Because the logical OR (
||) evaluates before the nullish coalescing operator (??), and"127.0.0.1"is a truthy string, the expression will always evaluate to"127.0.0.1"ifx-forwarded-foris not present. It completely skips checking thex-real-ipheader.Additionally, standard
x-forwarded-forheaders often contain a comma-separated list of IPs (e.g.,client_ip, proxy1, proxy2). The current logic does not split this string to retrieve the actual originating client IP, making it trivial for malicious users to spoof their IP address or bypass the rate limiter entirely.Expected Behavior
x-forwarded-forheader (taking the first IP in the comma-separated list).x-forwarded-foris absent, it should correctly fall back tox-real-ip."127.0.0.1".Steps to Reproduce
x-forwarded-forheader but with anx-real-ipheader."127.0.0.1"instead of the providedx-real-ip.x-forwarded-for: 10.0.0.1, 192.168.0.1.Suggested Fix
Refactor the IP extraction logic to correctly prioritize and parse headers. For example:
```typescript
let ip = "127.0.0.1";
const forwardedFor = req.headers.get("x-forwarded-for");
if (forwardedFor) {
ip = forwardedFor.split(",")[0].trim();
} else {
ip = req.headers.get("x-real-ip") || "127.0.0.1";
}
```