Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Usage guide should mention stripping unexpected headers #26

Open
danielcompton opened this issue May 11, 2017 · 3 comments
Open

Usage guide should mention stripping unexpected headers #26

danielcompton opened this issue May 11, 2017 · 3 comments

Comments

@danielcompton
Copy link

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
Copy link
Owner

@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
Copy link
Author

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
Copy link

SociallyDev commented Jul 6, 2018

@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.

  • Custom header AND nested request element sources
  • Custom header/source ranking allowed
  • Easier to use

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
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants