Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Input library does not set IP correctly #227

Closed
kenjis opened this Issue · 6 comments

3 participants

@kenjis

Input library is prone to a spoofed IP attack. For example, we have a code that validates that the IP must be in the local host.

if ($this->input->ip_address() == '127.0.0.1') {
    $this->load->view('administration');
}

We have too the vulnerable code.

if (config_item('proxy_ips') != '' && $this->server('HTTP_X_FORWARDED_FOR') && $this->server('REMOTE_ADDR'))
{
  $proxies = ...
  $proxies = ...
  $this->ip_address = ...
}
elseif ($this->server('REMOTE_ADDR') AND $this->server('HTTP_CLIENT_IP'))
{
  $this->ip_address = $_SERVER['HTTP_CLIENT_IP'];
}
elseif ($this->server('REMOTE_ADDR'))
{
  $this->ip_address = $_SERVER['REMOTE_ADDR'];
}

If we have not configured proxy_ips directive, the execution flow arrives to first elseif statement. We can spoof the IP sending the non-standard Client-IP header.

The solution is to prioritize IP address.

if (config_item('proxy_ips') != '' && $this->server('HTTP_X_FORWARDED_FOR') && $this->server('REMOTE_ADDR'))
{
  $proxies = ...
  $proxies = ...
  $this->ip_address = ...
}
elseif ($this->server('REMOTE_ADDR'))
{
  $this->ip_address = $_SERVER['REMOTE_ADDR'];
}
elseif ($this->server('REMOTE_ADDR') AND $this->server('HTTP_CLIENT_IP'))
{
  $this->ip_address = $_SERVER['HTTP_CLIENT_IP'];
}

Off course, the comparison now does not has sense because the flow will never enter to other comparisons.

from: https://bitbucket.org/ellislab/codeigniter/issue/302/input-library-does-not-set-ip-correctly

@philsturgeon

Fixed!

@kenjis

merged pull request: #237

@kenjis kenjis referenced this issue from a commit in kenjis/CodeIgniter
kenjis Fix #227 - IP address spoofing by CLIENT-IP header 8375f7b
@kenjis kenjis referenced this issue from a commit in kenjis/CodeIgniter
kenjis Fix #227 - fix typo 67f6dc8
@kenjis

Pull request 237 is not completed, because it trusts CLIENT-IP header. But the header can easily be spoofed. If you have direct access to Internet.

You can easily check wheather can be spoofed or not.

Put the code below on your server

<?php if ( ! defined('BASEPATH')) exit('No direct script access allowed');

class Ip_address extends CI_Controller {

  function index()
  {
    echo $this->input->ip_address();
  }
}

Run the code below form you PC.

<?php

$uri = 'http://foo.example.com/ci/index.php/ip_address';

// send X-FORWARDED-FOR
$opt = array(
  'http'=> array(
    'header' => 'X-FORWARDED-FOR: 88.88.88.88',
  )
);
$context = stream_context_create($opt);
$page = file_get_contents($uri, 0, $context);
echo $page, "<br />\n";

// send CLIENT-IP
$opt = array(
  'http'=> array(
    'header' => 'CLIENT-IP: 99.99.99.99',
  )
);
$context = stream_context_create($opt);
$page = file_get_contents($uri, 0, $context);
echo $page, "<br />\n";

// send X-FORWARDED-FOR and CLIENT-IP
$opt = array(
  'http'=> array(
    'header' => array('X-FORWARDED-FOR: 88.88.88.88',
      'CLIENT-IP: 99.99.99.99',
    )
  )
);
$context = stream_context_create($opt);
$page = file_get_contents($uri, 0, $context);
echo $page, "<br />\n";

I'll send a new pull request: #371

@ktomk

Is there a configuration variable already where you can say if you want to trust proxy headers or not?

@kenjis

Do you mean use $config['proxy_ips'] and do not add a new config?

I'm not sure the case of using CLIENT-IP header. So I don't know $config['proxy_ips'] can be used. But at least Symfony2 has 2 parameters. So I added a new config.
https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpFoundation/Request.php#L449

@ktomk

No I was just asking if such a config exists, because I did not know. I'm new to CI.

I would recommend to stick with what Symfony2 offers, their HTTP stack is one of the best (if not the best) in the PHP arena. So if they have two config items, I'm pretty sure there is reason for that. Thanks for spotting these issues kenjis!

@narfbg narfbg referenced this issue from a commit
@narfbg narfbg Fix issues #227 and #907 b0fe0a9
@sviande sviande referenced this issue from a commit in sviande/CodeIgniter
@narfbg narfbg Fix issues #227 and #907 a3cbba2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.