pbojinov / request-ip

A Node.js module for retrieving a request's IP address on the server.
https://npmjs.com/package/request-ip
MIT License
826 stars 102 forks source link

Usage guide should mention stripping unexpected headers #26

Open danielcompton opened 7 years ago

danielcompton commented 7 years ago

request-ip goes down a list of different methods of providing an IP address, and picks the first one that works. If the user of this library doesn't filter out unknown/unexpected headers, this is quite a dangerous way of determining the IP address. If X-Client-IP wasn't filtered by the user's web server, then an attacker could set the X-Client-IP header themselves and request-ip would happily accept it as the 'real' IP.

A more secure design would be for the user to configure request-ip with the headers and request keys they expect to get the IP from, and to only use them.

pbojinov commented 7 years ago

@danielcompton that's a good use case. Would you mind posting some code examples or submitting a pull request with how you'd think this should work?

danielcompton commented 7 years ago
const requestIp = require('request-ip');
requestIpSafe = requestIp.configure(['X-Client-IP', 'X-Forwarded-For', 'req.socket.remoteAddress'])

// inside middleware handler
const ipMiddleware = function(req, res, next) {
    const clientIp = requestIpSafe.getClientIp(req); 
    next();
};

requestIp.configure would take an array of IP detectors, set them with that ordering, and return a function to lookup the IP address using only those detectors.

SociallyDev commented 6 years ago

@pbojinov I had the same problem so I wrote an optimized version of request-ip that allows for custom sources from headers as well as the req var.

I'd love to improve on your work, I can submit a pull request, or alternatively help you manage request-ip if you'd like me to, just add me as a collaborator!

I'm @SociallyDev everywhere if you'd like to talk to me.

Here's the code:

/*
Fetches an IP from the request var.
The first valid IP source in the array is used.
NOTE: Only use trusted header sources because anyone can send any headers.
*/
const is = require("is_js")
function getIP(req, customSources) {
  var ip, source,
  sources = ["x-client-ip", "x-forwarded-for", "cf-connecting-ip", "true-client-ip", "x-real-ip", "x-cluster-client-ip",
             "x-forwarded", "forwarded-for", "forwarded", "req.connection.remoteAddress", "req.connection.socket.remoteAddress",
             "req.socket.remoteAddress", "req.requestContext.identity.sourceIp"]

  //Load custom sources.
  if(customSources) { sources = customSources }

  //Loop through the sources.
  for(var key in sources) {
    source = sources[key]
    //Separately handle req sub-elements.
    if(source.includes("req.")) {
      ip = getElement(req, source.replace("req.", ""))
      if(is.ip(ip)) {
        return ip
      }
    }
    else {
      if(!req.headers) { continue }
      //Look in req headers.
      ip = req.headers[source.toLowerCase()]
      //Only get client's IP from X-Forwarded-For header (It also includes proxy IPs).
      if(source.toLowerCase() == "x-forwarded-for" && ip) {
        ip = ip.split(",")[0].trim()
        //Azure adds port number to the IP so make sure only IP is returned.
        if(ip.includes(":")) {
          var split = ip.split(":")
          if(split.length === 2) {
            ip = split[0]
          }
        }
      }
      if(is.ip(ip)) {
        return ip
      }

    }
  }
  return false
}

/*
Gets a nested result from an array using dotted keys.
*/
function getElement(array, dottedKey) {
  var keys = dottedKey.split(".")
  var i = 0
  while(i < keys.length) {
    array = array[keys[i]]
    if(!array) { return false }
    i++
  }
  return array
}