Project

General

Profile

Bug #459

ip2long (and possibly others) return bogus values on 64 bit

Added by Chris Buechler over 9 years ago. Updated over 9 years ago.

Status:
Closed
Priority:
Low
Assignee:
-
Category:
-
Target version:
Start date:
03/29/2010
Due date:
% Done:

0%

Estimated time:
Affected Version:
2.0
Affected Architecture:

Associated revisions

Revision 96033063 (diff)
Added by Erik Fonnesbeck over 9 years ago

Various fixes to usage of ip2long, long2ip, and negated subnet masks, mostly affecting 64-bit. Ticket #459

History

#1 Updated by Ermal Lu├ži over 9 years ago

One workaround for this is to use pack/unpack or use filter_validate($ip, FILTER_VALIDATE_IP)

#2 Updated by Jim Pingle over 9 years ago

As noted by someone on the mailing list, this also affects input validation on things such as the DHCP server that just need to be checked again after fixing the core issue.

#3 Updated by Yehuda Katz over 9 years ago

This should have been fixed in the PHP5 source as of last April (2009).
If the issue is still there, maybe a bug report to PHP is in order.
See http://bugs.php.net/47365

#4 Updated by Yehuda Katz over 9 years ago

Sorry wrong link.
I can't find the right one now, but 32-bit will return signed int, and 64-bit will return unsigned.
There was a patch for it, but I am not sure what the status is.

#5 Updated by Michael Welter over 9 years ago

From what I've read it this is not considered a bug from PHP's perspective since it is documented "Because PHP's integer type is signed, and many IP addresses will result in negative integers" See [[http://php.net/ip2long]]

For now I've gone through all the PHP pages on my system and wrapped ip2long calls w/ unpack('l',pack('l',ip2long(...))) but ultimately I think it would be a better idea to encapsulate all IPv4 code in an ipv4 class with address(), netbits(), netmask(), inverse(), network(), broadcast(), ipcompare(), ...

#6 Updated by Scott Ullrich over 9 years ago

Can you provide us a patch that wraps all ip2long calls with unpack?? We can get that committed. Probably will not make sense to redo the functions until we add ipv6 support.

#7 Updated by Jim Pingle over 9 years ago

It would probably be better to make our own ip2long function that does it right, and then change all our calls to use it instead. I don't think PHP will let you overload it though so it would involve renaming all instances of ip2long with whatever we make.

#8 Updated by Michael Welter over 9 years ago

Sure, let me test tonight more thoroughly and I'll submit tomorrow.

#9 Updated by Erik Fonnesbeck over 9 years ago

The issue on 64-bit is likely that all the high 32 bits are set from sign extension because it considers it negative, interfering with bitwise comparisons when one IP address being compared has those high 32 bits set, so I think ip2long($ip) & 0xFFFFFFFF should work. If you want a function that does this, it could be something like this:

function ip2long32($ip) {
    return ( ip2long($ip) & 0xFFFFFFFF );
}

#10 Updated by Erik Fonnesbeck over 9 years ago

I've also seen places where subnet masks are negated with the bitwise not operator. In these places, we may also need to use & 0xFFFFFFFF on either the negated mask or the resulting address. Depending on the behavior of long2ip, it may also be necessary to use that bit mask to truncate certain calculated addresses to 32 bits before passing to long2ip. I think long2ip may just truncate to 32-bits on the PHP version used, but I'm not 100% sure.

#11 Updated by Erik Fonnesbeck over 9 years ago

  • Status changed from New to Feedback

I've made changes throughout the code to various things that used ip2long and long2ip that should fix it. I only changed the parts where there could be an issue and left the rest alone.

#12 Updated by Michael Welter over 9 years ago

Thanks Erik, I'll test once it is in the nightly build.

#13 Updated by Michael Welter over 9 years ago

Thanks again, I've verified it is now working on my 64-bit system.

#14 Updated by Chris Buechler over 9 years ago

  • Status changed from Feedback to Closed

Also available in: Atom PDF