Fixed BannedUsers spam check matches all users if Ip::convertIpStringToBinary returns false

Steffen

Well-known member
Affected version
2.2.9
In the method XF\Spam\Checker\BannedUsers::check, if calling Ip::convertIpStringToBinary() returns false (*) then the finder call ->where('ip', $ip) builds the condition `xf_ip`.`ip` = 0 which MySQL evaluates to true for nearly all strings (see e.g. https://stackoverflow.com/questions/22080382/mysql-why-comparing-a-string-to-0-gives-true). As a result, the query matches all banned users (not only those who have a matching IP address).

(*) What XenForo passes to XF\Spam\Checker\BannedUsers::check is the result of XF\Http\Request::getIp with $allowProxied = true, which makes me think that the client may have passed and invalid (or maybe even intentionally spoofed?) value in the "X-Forwarded-For" header. Maybe XenForo should not trust the "X-Forwarded-For" header as it can be easily spoofed to bypass the check (or make it fail in the opposite way like explained above)?
 

XF Bug Bot

XenForo bug fixer bot
Staff member
Thank you for reporting this issue, it has now been resolved. We are aiming to include any changes that have been made in a future XF release (2.2.10).

Change log:
Ensure spam checks handle cases where IP binary conversion fails, and do not trust proxied IP headers
There may be a delay before changes are rolled out to the XenForo Community.
 
Top