Skip to content

Security vulnerability: Forwarded does not pick the rightmost for entry, even in SecureClientIp #32

@demurgos

Description

@demurgos

The current impl to extract the ip from the Forwarded header is the following:

impl MultiIpHeader for Forwarded {
    const HEADER: &'static str = "Forwarded";

    fn ips_from_header_value(header_value: &str) -> Vec<IpAddr> {
        use forwarded_header_value::{ForwardedHeaderValue, Identifier};

        let Ok(fv) = ForwardedHeaderValue::from_forwarded(header_value) else {
            return Vec::new();
        };
        fv.iter()
            .filter_map(|fs| fs.forwarded_for.as_ref())
            .filter_map(|ff| match ff {
                Identifier::SocketAddr(a) => Some(a.ip()),
                Identifier::IpAddr(ip) => Some(*ip),
                _ => None,
            })
            .collect()
    }
}

The first filter_map call is right, it's just there to extract the for entries. The second filter map however is problematic. It drops values that are not recognized by this lib! This means that if the header is Forwarded: for="1.2.3.4",for="_obfuscated" then this lib will pick the wrong value.

This is especially problematic since the for entry in the Forwarded RFC (RFC7239) recommends proxies to use an obfuscated entry instead of an address by default. If a reverse proxy follows the spec and uses an obfuscated entry, this lib will extract a client-controlled value!

If there is no way to extract the client ip securely but the handlers requires it, the extractor should reject the request instead of passing potentially insecure data.

I did not review the other handlers, but I recommend checking them for similar issues.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions