Fixed Range bans incorrectly apply to certain addresses (rcon addip, banIP.dat)

Posts
278
Likes
361
What went wrong?
As the title suggests I recently got approached by a player that claimed he had been incorrectly banned.
After checking his IP address I realized that in certain situations it incorrectly range bans when I don't want it to.
I'm not sure if this is an oversight or a bug but in any case it results in me not being able to range ban some ranges without negatively impacting other players.

Scenario
Banned player Bob has address 350.0.371.385
Innocent player Billy has address 350.432.371.392

Bob is playing on a mobile network and has range 350.0.371.0 - 350.0.371.255
I add range 350.0.371.0 to the ban list.
Bob is now banned.
Billy is now banned.

What should have happened?
I should be able to range ban Bob without adversely affecting players like poor Billy

Edit: Clarified the mobile network range
 
Last edited:

Spaghetti

Floating in the void
R2D2
Movie Battles II Team Retired
Posts
1,637
Likes
1,633
Those are invalid IP ranges so it's a little difficult to parse your example. The numbers aren't arbitrary.

In any case the problem here is that 0 is being interpreted as a wildcard. Filtering 123.0.123.0 will ban anything that matches the first and third octet.
 
Posts
278
Likes
361
Those are invalid IP ranges so it's a little difficult to parse your example. The numbers aren't arbitrary.

In any case the problem here is that 0 is being interpreted as a wildcard. Filtering 123.0.123.0 will ban anything that matches the first and third octet.
To clarify a bit more maybe. What's important is that its possible for a player to have an IP address like XXX.0.XXX.XXX and banning them like XXX.0.XXX.0 will, like you said, ban anything in range XXX.0.XXX.0 - XXX.255.XXX.255. Making it impossible to target the desired range XXX.0.XXX.0 - XXX.0.XXX.255
 

Spaghetti

Floating in the void
R2D2
Movie Battles II Team Retired
Posts
1,637
Likes
1,633
Right, 0 should be valid as part of the network address. I have a solution planned but it will break backward compatibility with existing range bans.
 

Defiant

Nerd
Project Leader
Movie Battles II Team
Code Leader
Posts
1,007
Likes
1,451
Right, 0 should be valid as part of the network address. I have a solution planned but it will break backward compatibility with existing range bans.

We can do it without breaking backwards compatibility. It's not a big deal.
 

Spaghetti

Floating in the void
R2D2
Movie Battles II Team Retired
Posts
1,637
Likes
1,633
We can do it without breaking backwards compatibility. It's not a big deal.
It kind of depends on what you mean by that. I was talking about how existing entries are parsed from the .dat. There's no information stored on whether something is intended to be part of a network address or a range, so behavior will change.

We could assume 0 in the last octet is a range but that poses some risk in edge cases such as a /23 network or anything else where 0 could be part of the host range. That's not super common but I think it would be better just to make ranges explicit. I've already coded a fix but still need to test it more.
 
Last edited:
Posts
278
Likes
361
If I can sneak in a little feature request. It would be nice to have more control of ranges. Sometimes a naughty player has a range like: XXX.XXX.125.0 - XXX.XXX.180.255. 👀
 

Spaghetti

Floating in the void
R2D2
Movie Battles II Team Retired
Posts
1,637
Likes
1,633
If I can sneak in a little feature request. It would be nice to have more control of ranges. Sometimes a naughty player has a range like: XXX.XXX.125.0 - XXX.XXX.180.255. 👀
As nice as that might be, I don't really have the will to implement proper input and saving of subnet masks when I'm wanting to focus on gameplay features.
 

Defiant

Nerd
Project Leader
Movie Battles II Team
Code Leader
Posts
1,007
Likes
1,451
It kind of depends on what you mean by that. I was talking about how existing entries are parsed from the .dat. There's no information stored on whether something is intended to be part of a network address or a range, so behavior will change.

We could assume 0 in the last octet is a range but that poses some risk in edge cases such as a /23 network or anything else where 0 could be part of the host range. That's not super common but I think it would be better just to make ranges explicit. I've already coded a fix but still need to test it more.

There are a few ways to save the state - but it boils down to somehow doing a one off loading old IPs and saving them with some special character in place of any 0's which means "Any IP matching this octet". There are a load of differnt options with various advantages and draw backs.

It would just be nice if we didnt create a minor headache for servers by appearing to unban people who are caught at present by the over zealous filtering. This system appears to be the same (or substantially the same) as the baseJK ban system - and if this has been an issue in the past it has been exceedingly rare. The principle of least change would seem to apply here.
 

Spaghetti

Floating in the void
R2D2
Movie Battles II Team Retired
Posts
1,637
Likes
1,633
There are a few ways to save the state - but it boils down to somehow doing a one off loading old IPs and saving them with some special character in place of any 0's which means "Any IP matching this octet". There are a load of differnt options with various advantages and draw backs.

It would just be nice if we didnt create a minor headache for servers by appearing to unban people who are caught at present by the over zealous filtering. This system appears to be the same (or substantially the same) as the baseJK ban system - and if this has been an issue in the past it has been exceedingly rare. The principle of least change would seem to apply here.
I thought about doing that but we can't be sure existing 0s are intended as wildcards either given this bug. A server owner would have to get contacted by a player that got caught up in an accidental ban to even have a chance of realizing it was a problem so it's hard to say how rare it actually is. Aside from intentional range bans going out of scope, there's probably instances of it happening just from a regular ban on a player with a 0 in their IP too.

EDIT: I don't think it's a good idea but I've added code to handle migrating the bugged state.
 
Last edited:
Posts
238
Likes
225
As nice as that might be, I don't really have the will to implement proper input and saving of subnet masks when I'm wanting to focus on gameplay features.
Understandable

Knowing a fix for the OP message is coming is already a fresh breath !
Thanks you for coding that :)
 
Top