I'm working on getting our Magento v1.9 instance to pass a compliance audit, and one of the outstanding items is in regards to the PHP ip2long function.
Details:
Basically, you can append a malicious string on the end of an IP passed to ip2long, and instead of failing, it will return the "long" value for the IP thats on the beginning of the string. This only seems to be an issue if someone improperly uses ip2long as a means of IP format verification.
Example:
$ php -r 'echo ip2long("1.1.1.1").PHP_EOL;'
16843009
$ php -r 'echo ip2long("1.1.1.1".chr(0)."bad-stuff").PHP_EOL;'
16843009
And I can see that the ip2long is used in a few places:
$ grep -RnH ip2long base current
base/includes/src/Mage_Core_Helper_Http.php:149: return $ipToLong ? ip2long($this->_remoteAddr) : $this->_remoteAddr;
base/includes/src/Mage_Core_Helper_Http.php:164: return $ipToLong ? ip2long($address) : $address;
base/includes/src/Zend_Validate_Ip.php:147: $ip2long = ip2long($value);
base/includes/src/Zend_Validate_Ip.php:148: if($ip2long === false) {
base/includes/src/Zend_Validate_Ip.php:152: return $value == long2ip($ip2long);
base/includes/src/Zend/Validate/Ip.php:147: $ip2long = ip2long($value);
base/includes/src/Zend/Validate/Ip.php:148: if($ip2long === false) {
base/includes/src/Zend/Validate/Ip.php:152: return $value == long2ip($ip2long);
base/lib/Zend/Validate/Ip.php:147: $ip2long = ip2long($value);
base/lib/Zend/Validate/Ip.php:148: if($ip2long === false) {
base/lib/Zend/Validate/Ip.php:152: return $value == long2ip($ip2long);
base/app/code/local/Mage/Core/Helper/Http.php:149: return $ipToLong ? ip2long($this->_remoteAddr) : $this->_remoteAddr;
base/app/code/local/Mage/Core/Helper/Http.php:164: return $ipToLong ? ip2long($address) : $address;
current/modules/Mage/Core/Helper/Http.php:149: return $ipToLong ? ip2long($this->_remoteAddr) : $this->_remoteAddr;
current/modules/Mage/Core/Helper/Http.php:164: return $ipToLong ? ip2long($address) : $address;
From what I can tell, one would only be vulnerable...
- If the application is developed to give precedence to the X-Forwarded-For header value over the REMOTE_ADDR value, which it looks like Magento does
- If the ip2long is improperly used to validate an IP address, instead of being used to store the result in a database. Then that "validated" string is used in a query. I don't see anywhere in Magento that the ip2long is used only to validate.
Questions:
- Both of the above conditions would need to be met for this to be a security issue, and from what I can tell, Magento only meets #1, not #2, but if someone could verify that for me, that would be appreciated.
- Our Magento instance doesn't have any use for X-Forward-For, as the traffic to it isn't sent over a reverse proxy or anything else that would utilize this header. So I was thinking as a quick fix, I could just add a rewrite rule in the Apache VHost to deny any requests that have that header set... Does anyone see any issues with this?