dsoares / roundcube-rcguard

Roundcube plugin to enforce reCAPTCHA for users who have too many failed logins.
24 stars 10 forks source link

Fix 'Unsupported operand types: string & bool' with IPv6 clients on PHP 8+ #46

Closed bratkartoffel closed 1 year ago

bratkartoffel commented 2 years ago

When a client connects to a roundcube instanced with rcguard active, the request aborts with a HTTP 500 error due to an exception.

PHP Fatal error:  Uncaught TypeError: Unsupported operand types: string & bool in /var/www/htdocs/plugins/roundcube-rcguard-1.3.2/rcguard.php:404
Stack trace:
#0 /var/www/htdocs/plugins/roundcube-rcguard-1.3.2/rcguard.php(53): rcguard->get_client_ip()
#1 /var/www/htdocs/program/lib/Roundcube/rcube_plugin_api.php(105): rcguard->init()
#2 /var/www/htdocs/program/include/rcmail.php(153): rcube_plugin_api->init()
#3 /var/www/htdocs/program/include/rcmail.php(86): rcmail->startup()
#4 /var/www/htdocs/index.php(43): rcmail::get_instance()
#5 {main}
  thrown in /var/www/htdocs/plugins/roundcube-rcguard-1.3.2/rcguard.php on line 404

Code to reproduce (extracted from the plugins code):

cat >/tmp/test.php <<"EOF"
<?php
$prefix = 64;
$client_ip = "2001:db8:abcd:0012:1234::3";
$mask_string = str_repeat('1', $prefix) . str_repeat('0', 128 - $prefix);
$mask_split = str_split($mask_string, 16);
foreach ($mask_split as $item) {
  $item = base_convert($item, 2, 16);
}
$mask_hex = implode(':', $mask_split);

var_dump(array(
  inet_pton($client_ip),
  inet_pton($mask_hex),
  inet_pton($client_ip) & inet_pton($mask_hex)
));
EOF

php /tmp/test.php

Prior PHP 8 (e.g. with 7.4), the '&' returns an int and issues just a warning (A non-numeric value encountered in /tmp/test.php on line 14).

pbiering commented 1 year ago

This PR may fix the exception on PHP8 and warning on PHP7, but the whole whitelist code is not supporting IPv6 at all at the moment. I will try to fix this now and submit a dedicated PR.

pbiering commented 1 year ago

this PR is now fixed and superseeded by https://github.com/dsoares/roundcube-rcguard/pull/49

@bratkartoffel : please check the new PR and potentially close yours

bratkartoffel commented 1 year ago

the other PR looks fine as far as i can tell, closing this. Thanks!