Page MenuHomePhabricator

IP::isInRange() returns true on nonsense input
Closed, ResolvedPublic

Description

In AbuseFilter

is_in_range(ip, range)

will give true, if ip is not an ip and range is not a range.

The reason seems to be that

IP::isInRange($ip, $range)

in mediawiki-core(?), see https://doc.wikimedia.org/mediawiki-core/master/php/IP_8php_source.html#l00637, uses

strcmp(. , .) >= 0 && strcmp(. , .) <= 0,

which is always true, if the params are not defined, because strcmp results in 0.

This can lead to big problems, if somebody makes a minor error in using the AbuseFilter extension, e.g.

is_in_range(user_name, "127.0.1/16")
is_in_range("127.0.1/16", user_name)
is_in_range(user_name, "wtf")

Event Timeline

seth raised the priority of this task from to Medium.
seth updated the task description. (Show Details)
seth subscribed.

We could try to check the IP range at validation level and do IP::isIPAddress( $IP ) && IP::isInRange( $ip, $range ) during evaluation instead of just checking isInRange?

Change 266595 had a related patch set uploaded (by MtDu):
Fix IP::isInRange() returning true on nonsensical outputs

https://gerrit.wikimedia.org/r/266595

Not sure if this works. (letting result = null) Let me know your opinions.
Thanks,
MtDu

Change 275544 had a related patch set uploaded (by Glaisher):
Don't allow invalid IP ranges to be entered in ip_in_range()

https://gerrit.wikimedia.org/r/275544

Change 275564 had a related patch set uploaded (by Glaisher):
Add note that IP::isInRange() can return unexpected results for invalid args

https://gerrit.wikimedia.org/r/275564

Change 275564 merged by jenkins-bot:
Add note that IP::isInRange() can return unexpected results for invalid args

https://gerrit.wikimedia.org/r/275564

Change 266595 abandoned by MtDu:
Check whether IP is valid in ip_in_range()

Reason:
Per @TTOs comment.

https://gerrit.wikimedia.org/r/266595

Change 275544 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Don't allow invalid IP ranges to be entered in ip_in_range()

https://gerrit.wikimedia.org/r/275544