bugintermediateP1internal/middleware/ratelimit.goLevel 1 · Fix a bug15 XP45–60 minutes

X-Forwarded-For trusts the wrong (spoofable) hop

Nobody has started this yet — be first.

Business impact

Rate limiting exists to protect the business from abuse -- credential stuffing, scraping, a single bad actor consuming disproportionate infrastructure cost. A key-derivation bug that lets an attacker pick a fresh "identity" per request defeats that protection entirely for every rule keyed by IP. This is a security control that looks like it's working (dashboards look normal, denials happen) while silently protecting against nothing -- exactly the kind of gap that shows up in a security audit or an incident postmortem.

Problem

KeyByIP's doc comment says it prefers the leftmost hop in X-Forwarded-For (set by a trusted upstream proxy). The implementation reads a different slice index than documented.

Current behavior

For a request with header X-Forwarded-For: 203.0.113.7, 10.0.0.5 (client IP first, each proxy appends its own hop after), KeyByIP returns 10.0.0.5 -- an internal infrastructure hop address, not the client.

Expected behavior

KeyByIP should return 203.0.113.7, the leftmost entry, which by convention is the original client address as seen by the first proxy in the chain.

Steps to reproduce

req := httptest.NewRequest("GET", "/", nil) req.Header.Set("X-Forwarded-For", "203.0.113.7, 10.0.0.5") key := middleware.KeyByIP(req) // got: "10.0.0.5" // want: "203.0.113.7"

Why this matters

This is a rate-limit bypass, not a cosmetic bug. If the key used for rate limiting is influenced by a hop an attacker can control, and the code reads a different position than the one the trusted proxy actually appends, an attacker can manipulate the number of comma-separated segments they send to pick a fresh identity per request and evade the limiter entirely.

Suggested approach

Look at exactly which slice index is being read out of the comma-split header value, and compare it against what the doc comment promises ("leftmost"). Fix the index, not the splitting/trimming logic, which is otherwise correct.

Acceptance criteria

KeyByIP on a request with a multi-hop X-Forwarded-For returns the first (leftmost) entry, trimmed of whitespace
Single-hop headers and the no-header fallback to RemoteAddr are unaffected
go test ./test/ -run TestTask04 -v passes

Verification

go test ./test/ -run TestTask04 -v

Hints (0/2)

Try it without hints first — the reading is the exercise.

Working on this ticket

Work on a branch named for the ticket — that's what you'll submit.

Branch off your fork

$git checkout -b fix/forwarded-for-wrong-hop

Fix it and commit

Meet every acceptance criterion, and add a test that would have caught this.

Push the branch

$git push -u origin fix/forwarded-for-wrong-hop

Submit it below

Paste your fork URL and the branch name, with a short write-up of the root cause.

Submit your fix

Questions

Stuck on something?

Ask about anything unclear in the ticket — the maintainer and anyone who has solved it can answer. Please don't post full solutions.

No questions yet. If something in this ticket reads ambiguously, you are probably not the only one — ask.

The project

Rate Limiter Service

Token bucket, sliding window and leaky bucket, in-memory and Redis-backed — fifteen ways the production version of each quietly breaks.

Fork on GitHub

Download

Setup

Rate Limiter Service

git clone <your-fork>
cd GolangDevTraining/ratelimiter
docker compose up --build   # app + redis + redis-commander, hot-reloading on save

The service runs on http://localhost:8080 (Redis on 6380, Redis Commander on 8081). Verify with curl localhost:8080/healthz and curl localhost:8080/api/v1/rules.

Work the tickets in tasks/ (ordered easy → hard); each names one Go test in test/:

go test ./test/ -run TestTask01 -v   # a single ticket
./test/run_all.sh                   # all 15, clean pass/fail summary

Level 1

Fix a bug

Read existing behaviour, correct it.