IP Whitelist function

Question

I've just created a function to check if your IP is within an array of IPs. The IP array can contain ranges like 192.168/170.50.0/255. I have a working example, but it seems a bit clunky to me.

Is there anything wrong with my method, or anything I could improve?

Link to Ideone example.

<?php
 
function checkIpWhitelist($whitelist, $yIp){
 
 
    $ips = $whitelist;
    $ySplitIp = explode('.', $yIp);
 
    $found = null;
 
    // Loop each IP.
    foreach($ips as $ip){
 
        // Explode it into parts.
        $splitIp = explode('.', $ip);
 
        // Each part.
        foreach($splitIp as $index => $part){
 
            // If this IP has failed a part, skip the rest of it.
            if($found === false){
                continue;
            }
 
            // Default to an array of just the part value.
            $range = array($part);
 
            // If the part contains a /, it's a range.
            if(strpos($part, '/') !== false){
 
                // Split the range.
                $range = explode('/', $part);
 
                // Make a new array, containing the full range.
                $range = range($range[0], $range[1]);
            }
 
            // If the part is in the range.
            if(in_array($ySplitIp[$index], $range)){
                $found = true;    
            } else {
                $found = false;
            }
        }
 
        // If $found is true, all IP sections are matched. If not, reset $found to null.
        if($found === true){
            return true;
        } else {
            $found = null;
        }
    }
 
    return $found;
}
 
 
$yIp = '192.168.59.3';
$whitelist = array(
    array(
        '192.168.60.1',
        '192.168.0/50.3',
        '192.160/170.59.4'
    ),
    array(
        '192.168.60.1',
        '192.168.0/50.3',
        '192.160/170.59.3',
        '192.168.59.3'
    ),
    array(
        '192.168.60.1',
        '192.168.0/50.3',
        '192.160/167.59.3',
        '190/193.160/169.50/60.0/5',
        '192.168.59.3'
    )
);
 
foreach($whitelist as $list){
    var_dump("\n\n" . (checkIpWhitelist($list, $yIp) ? 'IP in range!' : 'IP not in range') . "\n\n");
}
?>

The examples return:

IP not in range

IP in range!

IP in range!

Answer 1

It took me a while until I understood your range notation. Consider switching to CIDR syntax. Networks are routed using CIDR so you usually can express ownership of an IP address by CIDR notation.

Another thing to note is that your code does not support IPv6. IPv6 will become more and more important over the next years. I recommend supporting it right now to be safe!

---

Now to your code:

1. Your function does several things at once. It loops through the provided ranges and checks each range. This should be separated into two functions: One reusable function that checks whether the IP address matches the whitelist entry and another one that calls the first function for each whitelist entry:

   function checkIpWhitelist($ranges, $ip) {
       foreach ($ranges as $range) {
           if (isIpInRange($range, $ip)) return true;
       }
   
       return false;
   }
   

2. Your algorithm indeed is overly complex and also has an unnecessary overhead because of that range call. range will create an array using that range, this takes time and memory!

   I suggest normalizing the provided whitelist entry first: Convert any non-range to a range containing only that number and convert it to a data structure that is easier to handle than a string (array containing integers). Note that I am using array_map for that. In my opinion this functional programming style is easier to read and understand. You can easily write it as a normal foreach loop, though!

   function isIpInRange($rangeString, $ip) {
       $range = array_map(function ($item) {
           $parts = explode('/', $item);
           if (count($parts) == 1) $parts[1] = $parts[0];
   
           return array_map('intval', $parts);
       }, explode('.', $rangeString));
   

   Provided the user put in a syntactically correct range $range will be an array containing 4 arrays with two integers (start and each of the range) each. We can now check each part. I put a not around the whole expression in the if, as I find it more natural to read (“unless $ip[$i] is in the range the result is false”)

       $ip = explode('.', $ip);
       for ($i = 0; $i < 4; $i++) {
           if (!($range[$i][0] <= $ip[$i] && $ip[$i] <= $range[$i][1])) return false;
       }
       return true;
   }
   

3. Spice it up using sanity checking! Currently the algorithm accepts dumb inputs, such as -9.94143.255/123.1/2/3/3.5. We maybe should check that first. This is a security feature, isn't it? Validating IP addresses is easy using PHP:

   if (!filter_var($ip, FILTER_VALIDATE_IP)) {
       throw new InvalidArgumentException('Provide a valid ip address, please. "'.$ip.'" is not valid');
   }
   

   Validating your range is a little bit harder, but still possible. I suggest a regular expression for checking the basic structure and afterwards validating the ranges of the numbers. I am using Exceptions for error reporting here, but you can use anything you want.

   // before normalizing
   if (!preg_match('!^\d+(/\d+)?\.\d+(/\d+)?\.\d+(/\d+)?\.\d+(/\d+)?$!', $rangeString)) {
       throw new InvalidArgumentException('Provide a valid range, please. "'.$rangeString.'" is not valid');
   }
   
   // after normalizing
   for ($i = 0; $i < 4; $i++) {
       // negative and non-numbers caught by the regular expression
       if ($range[$i][0] > $range[$i][1] || $range[$i][0] > 255 || $range[$i][1] > 255) {
           throw new InvalidArgumentException('Provide a valid range, please. "'.$rangeString.'" is not valid');
       }
   }
   

Compiling that together we get these shiny two functions:

function checkIpWhitelist($ranges, $ip) {
    foreach ($ranges as $range) {
        if (isIpInRange($range, $ip)) return true;
    }

    return false;
}

function isIpInRange($rangeString, $ip) {
    if (!filter_var($ip, FILTER_VALIDATE_IP)) {
        throw new InvalidArgumentException('Provide a valid ip address, please. "'.$ip.'" is not valid');
    }

    if (!preg_match('!^\d+(/\d+)?\.\d+(/\d+)?\.\d+(/\d+)?\.\d+(/\d+)?$!', $rangeString)) {
        throw new InvalidArgumentException('Provide a valid range, please. "'.$rangeString.'" is not valid');
    }

    $range = array_map(function ($item) {
        $parts = explode('/', $item);
        if (count($parts) == 1) $parts[1] = $parts[0];

        return array_map('intval', $parts);
    }, explode('.', $rangeString));

    for ($i = 0; $i < 4; $i++) {
        // negative and non-numbers caught by the regular expression
        if ($range[$i][0] > $range[$i][1] || $range[$i][0] > 255 || $range[$i][1] > 255) {
            throw new InvalidArgumentException('Provide a valid range, please. "'.$rangeString.'" is not valid');
        }
    }

    $ip = explode('.', $ip);
    for ($i = 0; $i < 4; $i++) {
        if (!($range[$i][0] <= $ip[$i] && $ip[$i] <= $range[$i][1])) return false;
    }
    return true;
}