Skip to content

Verify explicitly that the request IP is a valid IPv4 address #19666

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

Closed
wants to merge 2 commits into from

Conversation

nesk
Copy link
Contributor

@nesk nesk commented Aug 18, 2016

Q A
Branch? 2.7
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets -
License MIT
Doc PR -

Take the following base code (the array is based on CloudFlare IP Ranges):

use Symfony\Component\HttpFoundation\IpUtils;

$ips = [
   "103.21.244.0/22",
   "103.22.200.0/22",
   "103.31.4.0/22",
   "104.16.0.0/12",
   "108.162.192.0/18",
   "131.0.72.0/22",
   "141.101.64.0/18",
   "162.158.0.0/15",
   "172.64.0.0/13",
   "173.245.48.0/20",
   "188.114.96.0/20",
   "190.93.240.0/20",
   "197.234.240.0/22",
   "198.41.128.0/17",
   "199.27.128.0/21",
   "2400:cb00::/32",
   "2405:8100::/32",
   "2405:b500::/32",
   "2606:4700::/32",
   "2803:f800::/32",
   "2c0f:f248::/32",
   "2a06:98c0::/29",
];

Before this PR, the following code would have returned true instead of the expected false value:

IpUtils::checkIp('blablabla', $ips);

This due to the ip2long function returning false for an invalid IP address, thus returning "00000000000000000000000000000000" with the following code:

sprintf('%032b', ip2long('blablabla'));

To fix this I simply check if the $requestIp variable contains a valid IP address.

@nicolas-grekas
Copy link
Member

could you add a test case please?

Test added for the c2b4525 commit
@nesk
Copy link
Contributor Author

nesk commented Aug 19, 2016

Done, added a new assertion checking if this line returns false:

IpUtils::checkIp('an_invalid_ip', '192.168.1.0/24');

@fabpot
Copy link
Member

fabpot commented Aug 19, 2016

Thank you @nesk.

fabpot added a commit that referenced this pull request Aug 19, 2016
…ess (nesk)

This PR was squashed before being merged into the 2.7 branch (closes #19666).

Discussion
----------

Verify explicitly that the request IP is a valid IPv4 address

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

Take the following base code (the array is based on [CloudFlare IP Ranges](https://www.cloudflare.com/ips/)):

```php
use Symfony\Component\HttpFoundation\IpUtils;

$ips = [
   "103.21.244.0/22",
   "103.22.200.0/22",
   "103.31.4.0/22",
   "104.16.0.0/12",
   "108.162.192.0/18",
   "131.0.72.0/22",
   "141.101.64.0/18",
   "162.158.0.0/15",
   "172.64.0.0/13",
   "173.245.48.0/20",
   "188.114.96.0/20",
   "190.93.240.0/20",
   "197.234.240.0/22",
   "198.41.128.0/17",
   "199.27.128.0/21",
   "2400:cb00::/32",
   "2405:8100::/32",
   "2405:b500::/32",
   "2606:4700::/32",
   "2803:f800::/32",
   "2c0f:f248::/32",
   "2a06:98c0::/29",
];
```

Before this PR, the following code would have returned `true` instead of the expected `false` value:

```php
IpUtils::checkIp('blablabla', $ips);
```

This due to the `ip2long` function returning `false` for an invalid IP address, thus returning `"00000000000000000000000000000000"` with the following code:

```php
sprintf('%032b', ip2long('blablabla'));
```

To fix this I simply check if the `$requestIp` variable contains a valid IP address.

Commits
-------

17e418c Verify explicitly that the request IP is a valid IPv4 address
@fabpot fabpot closed this Aug 19, 2016
This was referenced Sep 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants