19

Session Hijacking Prevention

It is good practice to bind sessions to IP addresses, that would prevent most session hijacking scenarios (but not all), however some users might use anonymity tools (such as TOR) and they would have problems with your service.

To implement this, simply store the client IP in the session first time it is created, and enforce it to be the same afterwards. The code snippet below returns client IP address:

$IP = (getenv ( "HTTP_X_FORWARDED_FOR" )) ? getenv ( "HTTP_X_FORWARDED_FOR" ) : getenv ( "REMOTE_ADDR" );

Source: https://www.owasp.org/index.php/PHP_Security_Cheat_Sheet#Session_Hijacking_Prevention

Seems to me this code is vulnerable to IP spoofing! Right?

Edit: AFIK, X_FORWARDED_FOR is an HTTP header that is very easy for clients to set to anything they want.

H M
  • 2,897
  • 6
  • 22
  • 21
  • 1
    From what I read in your quote, I'd think the same. Going to look at the context now... – Luc Apr 14 '13 at 09:02
  • Do you mean IP spoofing in the traditional sense (i.e. attacker starts a session with the server pretending to be another machine?) If so, then that's very unlikely to be successful over the Internet unless the attacker can use source routing (very unlikely) or the server has predictable TCP sequence numbers (extremely unlikely). If it's not that then you might want to update the question. – Rory McCune Apr 14 '13 at 09:04
  • 1
    @RoryMcCune He means that `getenv(forwarded-for)` can contain anything a user wants. – Luc Apr 14 '13 at 09:14
  • yep I saw your answer on that front, interesting. suggested edit was just to remove potential confusion over the term "IP spoofing" – Rory McCune Apr 14 '13 at 09:20
  • 2
    `X-Forwarded-For` is necessary when your site is behind a (trusted) reverse proxy, and should never be used otherwise. But the whole point is moot really - relying on IP address stability is a really bad idea that will break your site for many more users than just TOR. You can certainly use IP correlation as one input to a risk assessment tool, but making it a requirement to use the site at all is very silly. – bobince Apr 14 '13 at 14:20
  • i have IP correlation as an option in [my register and login system](https://github.com/ferchang/reg8log). there is a checkbox on my login form with the label 'Tie my login to my IP address'. also it can be made mandatory via the config file if needed (e.g for high security envs) - it can be turned on only for admin, only normal users, or both! Extreme flexibilty! BTW i don't use php session system but cookies. also user's ip is not stored on the server but combined with the login key with a hash function. – H M Apr 14 '13 at 19:46
  • In addition to not solving the problem of session hijacking, this code is going to break your site for legitimate traffic - remote_addr can change mid session! (although it's rare for the IP address to cross network ranges) – symcbean Apr 15 '13 at 14:11
  • Consider someone with a mobile phone (or tethered laptop) using your app while on a commuter train. Won't they have multiple IP addresses during the trip? – codingoutloud May 07 '15 at 14:24

3 Answers3

10

This code allows an attacker who knows the victim's X-Forwarded-For: header and the victim's session ID to login as that user. If the victim doesn't have an X-Forwarded-For: header, the attacker can put the victim's IP address in his header and the code will use that value as his legitimate IP address instead of his actual IP address. This is the vulnerability.

  • In a network-sniffing scenario, the attacker gets both of these values at the same time, hence adding this code does not protect against network sniffing.

  • In an XSS scenario, where the attacker injects javascript to steal the user's cookie, this is usually done by causing the victim's browser to make a request to an attacker-controlled web server with the cookie value appended as a GET parameter. The X-Forwarded-For: header of the victim would be supplied to the attacker in that request.

I haven't yet been able to think of any scenario where an attacker could obtain a victim's cookie but not be able to obtain the value of a legitimate X-Forwarded-For: header or, if no header was present, the victim's IP address.

So the answer is yes, as written, this code does nothing to improve security. It should trust the directly connected IP address only.

Note: If your web application server is behind a reverse web proxy (such as nginx, Varnish, HAProxy, mod_proxy, Amazon ELB and many others) that always adds the connecting IP address to an X-Forwarded-For header and makes a new connection to your web application server with its own IP address, you can "trust" part of the value of this header.

Modules such as mod_rpaf or mod_remoteip for Apache can be configured with a list of trusted proxy IP addresses and will replace the value of REMOTE_ADDR with the trusted part of the X-Forwarded-For header for connections coming from those addresses. This REMOTE_ADDR will be available in PHP as $_SERVER['REMOTE_ADDR] and will be logged in your Apache log files as the true connecting IP address.

If you don't do this (or the equivalent if you are not using Apache) then you will only ever see connections from one IP address, your reverse proxy server, and therefore all your sessions will contain the same IP address.

Ladadadada
  • 5,163
  • 1
  • 24
  • 41
  • 1
    _"the attacker can put the victim's IP address in his header and the code will use that value as his legitimate IP address."_ Hadn't thought of that yet. Good one. – Luc Apr 14 '13 at 09:32
9

I'd say you're right! They don't seem to have kept in mind that the HTTP header X-FORWARDED-FOR might return '; DROP TABLE users;--.

They suggest this in order to also lock sessions for people behind proxies, and name Tor as example. This is sort of stupid; Tor never forwards the original IP for obvious reasons. Moreover, it doesn't (shouldn't?) even look at the packet itself; it merely forwards it until it's out of the network. What's the use of a proxy if the original IP is forwarded anyway? Just lock it to the remote address, that's the only safe bet. Even if you were to validate the x-forwarded-for header as valid IPv4 or IPv6 address, it might still contain an arbitrary IP.

Below the example code on that page, they do think of local IPs and IPv6 (even though the IPv6 examples contain invalid addresses):

Keep in mind that in local environments, a valid IP is not returned, and usually the string :::1 or :::127 might pop up, thus adapt your IP checking logic.

But they should also have thought about that the header can contain arbitrary data. Nice find!

Since it's a wiki, I guess anyone with an account could edit the page. If not, you should contact them. This is indeed bad practice, and many people might even insert $IP unescaped into a database. It's usually not possible to spoof the REMOTE_ADDR environmental variable, so it should be safe under normal circumstances (though I wouldn't insert it unescaped even if that were the case), but the X-forwarded-for header breaks it all here.

Luc
  • 31,973
  • 8
  • 71
  • 135
  • 1
    What you're saying is that some more code that's not in the OWASP snippet *could* contain a vulnerability. It wouldn't matter that the `X-Forwarded-For` header contained `'; DROP TABLE users;--` if it's only stored in a local file, as PHP does by default. – Ladadadada Apr 14 '13 at 09:22
  • 3
    @luc I've changed this on the OWASP site, does the updated version look ok to you? – Rory McCune Apr 14 '13 at 09:28
  • 1
    Still a good answer, lots of people *do* store sessions and IP addresses in databases and it's important to understand that the `X-Forwarded-For` header can contain a malicious payload. – Ladadadada Apr 14 '13 at 09:39
  • 1
    Why would you mention SQL injection? That’s not the real problem. – Ry- Apr 14 '13 at 14:12
  • @minitech It's the first one that came to mind as exploitable vulnerability. Of course IP spoofing is also a problem. – Luc Apr 14 '13 at 14:49
2

This code is a security control, and as such, it must be evaluated against the threat it was intended to control. In this case, there is no special access being granted based on IP address. Instead, IP address is being used as an additional constraint on an already strong authorization mechanism (HTTP Cookies).

It is true that an attacker could spoof a user's IP through the X-Forwarded-For header and bypass this additional check, but that means that the attacker must not only know the user's session ID (cookie), but also the user's IP address. This makes exploitation of session-stealing XSS more difficult.

Consider the other ways this control could have been approached. First, if the control were not in place at all, then any session-stealing attack (XSS, traffic sniffing, SSL MITM, etc) would result in a fully-usable session for the attacker, with no extra work.

Instead, if the control were implemented without consulting the X-Forwarded-For header, but only HTTP_REMOTE_ADDR, then an attacker who is behind the same proxy (in a school or business, for instance) would not need any knowledge of the control or the user's internal IP to have a fully-usable session, since all outgoing HTTP sessions would appear to come from the same IP (that of the proxy).

By adding the check for X-Forwarded-For, the control requires an attacker to have knowledge of the user's IP address, and the ability to spoof it in the X-Forwarded-For header (not every attacker is completely unconstrained, so there is a reduction in risk. Not an elimination of risk, since we must assume there are some unconstrained attackers.)

One way to possibly strengthen this control would be to keep an extra bit of information: The source of the last IP address that was used. For instance, if the session were first granted and associated with an IP that came from HTTP_REMOTE_ADDR, the control could require that future traffic on that session must come from an IP gleaned from HTTP_REMOTE_ADDR, not X-Forwarded-For. This eliminates some chance for spoofing. The gains for implementing the check in reverse (preferring X-Forwarded-For over HTTP_REMOTE_ADDR) are probably not worth the effort, since it is much harder to spoof a real network address than an HTTP header.

bonsaiviking
  • 11,316
  • 1
  • 27
  • 50
  • i think it's better to use only REMOTE_ADDR, because, overall, there r more attackers in the world that have an ip address different from victim's and can't forge that; but if u used X-Forwarded-For then it's trivial to forge. – H M Apr 14 '13 at 19:30
  • @HM You're missing the point. By doing the check, the bar has already been raised. If your attacker has to spoof *anything*, you're imposing extra costs. User-Agent is also easily-spoofed, but should remain constant in a session and require reauth if it changes. You're just trying every possible way to invalidate a stolen session ID. – bonsaiviking Apr 15 '13 at 12:16
  • what u say is **adding** extra parameters/checks, not replacing an important parameter (because it's almost impossible for attackers to forge it in much more cases) with another parameter that is trivial to forge. The example code presented on OWASP is such a thing, not what u r saying. – H M Apr 17 '13 at 14:13
  • The page on OWASP says you should *invalidate* the session if there is a mismatch in previously observed parameters. It does **not** say you should use these parameters to *identify* a session. That's what Cookies are for. – bonsaiviking Apr 17 '13 at 14:23
  • i didn't ever claim that it says "use these parameters to identify a session"; what r u talking about?! that's completely irrelevant! what it says, and u can clearly see in the quoted text in my question, is using a code for getting the IP address of the client that effectively removes the necessity of matching REMOTE_ADDR for attackers and replaces it with an easily forgable parameter. – H M Apr 17 '13 at 14:39