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

update filter_var filters for ipv4 addresses to reflect rfc6890 #1954

Closed

Conversation

zghosts
Copy link
Contributor

@zghosts zghosts commented Jun 25, 2016

Looking into to bug#71745 reporting that the whole 127.0.0.0/8 should be caught by the FILTER_FLAG_NO_RES_RANGE filter I found the filter doesn't take all currently reserved ranges into account.
This pullrequest adds all the ranges defined in rfc6890 to the FILTER_FLAG_NO_RES_RANGE, this also means the private ranges have been added to the NO_RES_RANGE flag as they are technically also reserved.

In addition to this I added the 169.254 range to the FILTER_FLAG_NO_PRIV_RANGE flag as this is used for link_local in ipv4 effectively making it a private network.

I'm not sure it it should be added to 5.6 as it might introduce a bc break, but 7.1 or even 7.0 might be elligable.

@Fleshgrinder
Copy link
Contributor

Would be nice if the PHP manual is updated as well in case this gets merged.

@zghosts
Copy link
Contributor Author

zghosts commented Jun 26, 2016

I fully agree, if it gets merged I'm willing to update the manual accordingly so it reflects the changes in this pr.

@Majkl578
Copy link
Contributor

I already opened PR #1794 for 127.0.0.0/8 almost 4 months ago with no response at all.

@Fleshgrinder
Copy link
Contributor

I do not know who can help, maybe @nikic or @sgolemon know?

@jpauli
Copy link
Member

jpauli commented Jul 8, 2016

Merged

@Majkl578
Copy link
Contributor

Majkl578 commented Jul 8, 2016

Cool, I suppose we can expect this in 7.0.10 (or maybe even 7.0.9?), correct? 👍 @jpauli @weltling
I encountered this bug while using Symfony/Validator component which uses filter_var internally, so I'm basically waiting for this fix ever since. :)

@jpauli
Copy link
Member

jpauli commented Jul 8, 2016

Merged against 5.6 and up

@kemo
Copy link

kemo commented Aug 23, 2016

This also adds 192.168.0.0/16 into the reserved range? This should at least be mentioned in the documentation as it will break many tests that relied on the documentation for FILTER_FLAG_NO_RES_RANGE and it's a minor version change:

Fails validation for the following reserved IPv4 ranges: 
0.0.0.0/8, 169.254.0.0/16, 192.0.2.0/24 and 224.0.0.0/4. 
This flag does not apply to IPv6 addresses.

Code: https://3v4l.org/kkYAF

var_dump(filter_var('192.168.0.1', FILTER_VALIDATE_IP, FILTER_FLAG_NO_RES_RANGE));
var_dump(filter_var('192.168.255.255', FILTER_VALIDATE_IP, FILTER_FLAG_NO_RES_RANGE));
Output for 5.6.25, 7.0.10, 7.1.0beta1 - 7.1.0beta3
bool(false)
bool(false)
Output for 5.6.0 - 5.6.24, hhvm-3.10.0 - 3.12.0, 7.0.0 - 7.0.9, 7.1.0alpha1 - 7.1.0alpha3
string(11) "192.168.0.1"
string(15) "192.168.255.255"

@enov
Copy link

enov commented Aug 26, 2016

With this change, it is becoming harder write validations for IPs that allow private ones as well.

This simple function:

function ip($ip, $allow_private = TRUE)
{
    // Do not allow reserved addresses
    $flags = FILTER_FLAG_NO_RES_RANGE;
    if ($allow_private === FALSE)
    {
        // Do not allow private or reserved addresses
        $flags = $flags | FILTER_FLAG_NO_PRIV_RANGE;
    }

    return (bool) filter_var($ip, FILTER_VALIDATE_IP, $flags);
}

will become:

function ip($ip, $allow_private = TRUE)
{
    // FILTER_FLAG_NO_RES_RANGE includes FILTER_FLAG_NO_PRIV_RANGE
    $is_valid_public_ip = (bool) filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_NO_RES_RANGE);
    if ( ! $allow_private)
    {
        return $is_valid_public_ip;
    }
    // at this point we are allowing private IPs as well
    return (
        $is_valid_public_ip OR (
            (bool) filter_var($ip, FILTER_VALIDATE_IP) AND // is a valid IP
            ! (bool) filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_NO_PRIV_RANGE) // but it is private
        )
    );
}

@enov
Copy link

enov commented Aug 26, 2016

Also, FILTER_FLAG_NO_RES_RANGE constant value should reflect the idea that no_res_range includes no_priv_range

FILTER_FLAG_NO_RES_RANGE* = FILTER_FLAG_NO_RES_RANGE | FILTER_FLAG_NO_PRIV_RANGE

@jpauli
Copy link
Member

jpauli commented Sep 2, 2016

Fixing as of #2113

enov added a commit to kohana/core that referenced this pull request Oct 6, 2016
PHP 5.6.25, 5.6.26, 7.0.10, 7.0.11 include backward incompatible
bugfixes which later were reverted in the minor versions that followed.

See php/php-src#1954
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants