Security Issue: Bug in shift72/caddy-geo-ip causes RemoteAddr to be overwritten for the whole request

[adryd]

Not sure where or if this is appropriate to post, but I felt the need to notify people of this bug as it has security implications for those who use this plugin. The plugin has a fair amount of downloads on caddyserver.com

I’ve opened an issue Security: trust_header overwrites req.RemoteAddr globally · Issue #4 · shift72/caddy-geo-ip · GitHub, however I feel like users of the plugin are more likely to see it here.

when the trust_header directive is set in caddy-geo-ip, it overwrites req.RemoteAddr to the value of the header. This then gets passed down to other plugins and directives which share the same request.

example.com {
geo_ip {
        db_path /var/lib/GeoIP/GeoLite2-City.mmdb
        trust_header X-Forwarded-For
}

respond /test 200 {
        body "Remote Address:{remote_host}"
}
}

With the above config curl https://example.com/test -H "X-Forwarded-For: 1.1.1.1" would respond with Remote Address:1.1.1.1 showing that the value was changed outside of the geoip scope

This could allow people to bypass IP range restrictions and perhaps many other things I haven’t thought of.

[francislavoie]

Thanks for bringing that up.

FYI, Caddy will soon have an official and secure way to determine the client IP:

caddyhttp: Determine real client IP if trusted proxies configured