Refactor SetColor/Name anticheat

Turns out they were broken when I disabled them as I wasn't tracking
playerinfo's properly, but this is now fixed, so the checks can be
re-enabled.

One issue with the ColorLimits checks is that it didn't account for mods
that add extra colors, this is fixed by checking if the route followed
is plausible instead of mimicking client logic and only accepting a
single answer that it expected. A false negative is possible where the
host decides to wrap back to the front but there are still custom colors
available at the end of the list. This is acceptable because Impostor
doesn't currently know the amount of colors available and this issue
isn't major enough to ask all mod authors to declare their custom color
amount to the server.

As extra colors should now be handled by this code, it makes sense to
change the category of the unknown colors check to ProtocolExtensions.
For these mods an exemption of the ColorLimits category is no longer
necessary.

Note that mods that allow duplicate colors that do not use a custom rpc
to set colors can only be supported by disabling the ColorLimits
category, this was already the case and I don't think it can be unified
with the existing check.
This commit is contained in:
miniduikboot committed 2024-07-07 23:44:35 +02:00
1 parent 5afb2ca810
commit 609c30ffb4
2 files changed
+71 -29

No files matched your search

@@ -703,6 +703,7 @@ namespace Impostor.Server.Net.Inner.Objects
if (RequestedPlayerName.Any())
{
var expected = RequestedPlayerName.Dequeue();
var requested = expected;
if (Game.Players.Any(x => x.Character != null && x.Character != this && x.Character.PlayerInfo.PlayerName == expected))
{
@@ -723,12 +724,14 @@ namespace Impostor.Server.Net.Inner.Objects
if (name != expected)
{
// TODO fix
// if (await sender.Client.ReportCheatAsync(RpcCalls.SetName, CheatCategory.NameLimits, $"Client sent SetName with incorrect name, got '{name}', expected '{expected}'"))
// {
// await SetNameAsync(expected);
// return false;
// }
if (await sender.Client.ReportCheatAsync(
RpcCalls.SetName,
CheatCategory.NameLimits,
$"Client sent SetName with incorrect name, got '{name}', requested '{requested}', expected '{expected}'"))
{
await SetNameAsync(expected);
return false;
}
}
}
else
@@ -755,9 +758,9 @@ namespace Impostor.Server.Net.Inner.Objects
}
}
if ((byte)color > ColorsCount)
if ((byte)color >= ColorsCount)
{
if (await sender.Client.ReportCheatAsync(RpcCalls.CheckColor, CheatCategory.ColorLimits, "Client sent invalid color"))
if (await sender.Client.ReportCheatAsync(RpcCalls.CheckColor, CheatCategory.ProtocolExtension, "Client sent unknown color"))
{
return false;
}
@@ -778,9 +781,17 @@ namespace Impostor.Server.Net.Inner.Objects
}
}
if ((byte)color >= ColorsCount)
{
if (await sender.Client.ReportCheatAsync(RpcCalls.CheckColor, CheatCategory.ProtocolExtension, "Client sent unknown color"))
{
return false;
}
}
if (sender.IsOwner(this))
{
if (Game.Players.Any(x => x.Character != null && x.Character != this && x.Character.PlayerInfo.CurrentOutfit.Color == color))
if (Game.IsColorUsed(color, this))
{
if (await sender.Client.ReportCheatAsync(RpcCalls.SetColor, CheatCategory.ColorLimits, "Client sent a color that is already used"))
{
@@ -792,36 +803,56 @@ namespace Impostor.Server.Net.Inner.Objects
{
if (RequestedColorId.Any())
{
var expected = RequestedColorId.Dequeue();
var requested = RequestedColorId.Dequeue();
for (var colorOffset = 0; colorOffset <= ColorsCount; colorOffset++)
if (Game.IsColorUsed(color, this))
{
var possibleColor = (ColorType)((byte)(expected + colorOffset) % ColorsCount);
if (!Game.Players.Any(x => x.Character != null && x.Character != this && x.Character.PlayerInfo.CurrentOutfit.Color == possibleColor))
if (await sender.Client.ReportCheatAsync(RpcCalls.SetColor, CheatCategory.ColorLimits, "Client selected a color that is already used"))
{
expected = possibleColor;
break;
return false;
}
}
var startFrom = requested;
// Among Us wraps to the front if all colors between the requested and the final color are in use, check for this
if (requested > color)
{
// Among Us wrapped, so check all colors between requested and the final color
for (var c = (ColorType)requested; c < (ColorType)ColorsCount; c++)
{
if (!Game.IsColorUsed(c, this))
{
if (await sender.Client.ReportCheatAsync(
RpcCalls.SetColor,
CheatCategory.ColorLimits,
$"Client skipped color {c} that could be used, but wrapped instead. Player requested {requested} but was given {color}"))
{
await SetColorAsync(c);
return false;
}
}
}
if (colorOffset == ColorsCount)
// Start checking from the first color, fallthrough to normal case
startFrom = ColorType.Red;
}
// Check all colors between the requested color and the assigned color
for (var c = startFrom; c < color; c++)
{
if (!Game.IsColorUsed(c, this))
{
if (await sender.Client.ReportCheatAsync(RpcCalls.SetColor, CheatCategory.ColorLimits, "Client sent SetColor but all colors are already in use"))
if (await sender.Client.ReportCheatAsync(
RpcCalls.SetColor,
CheatCategory.ColorLimits,
$"Client skipped color {c} that could be used. Player requested {requested} but was given {color}"))
{
await SetColorAsync(expected);
await SetColorAsync(c);
return false;
}
}
}
// TODO fix
/*if (color != expected)
{
if (await sender.Client.ReportCheatAsync(RpcCalls.SetColor, CheatCategory.ColorLimits, "Client sent SetColor with incorrect color"))
{
await SetColorAsync(expected);
return false;
}
}*/
}
else
{
@@ -1011,7 +1042,7 @@ namespace Impostor.Server.Net.Inner.Objects
if (!await ValidateRole(RpcCalls.ProtectPlayer, sender, PlayerInfo, RoleTypes.GuardianAngel))
{
return false;
return false;
}
((InnerPlayerControl)target).Protect(this);
+11
View File
@@ -9,8 +9,10 @@ using Impostor.Api.Config;
using Impostor.Api.Events.Managers;
using Impostor.Api.Games;
using Impostor.Api.Innersloth;
using Impostor.Api.Innersloth.Customization;
using Impostor.Api.Innersloth.GameOptions;
using Impostor.Api.Net;
using Impostor.Api.Net.Inner.Objects;
using Impostor.Api.Net.Manager;
using Impostor.Api.Net.Messages.S2C;
using Impostor.Server.Events;
@@ -131,6 +133,15 @@ namespace Impostor.Server.Net.State
}
}
/// <summary>Check if there are players using a color.</summary>
/// <param name="color">The color to check for.</param>
/// <param name="exceptBy">Exempt a player from being checked.</param>
/// <returns>True if there is player other than exceptBy that uses that color.</returns>
internal bool IsColorUsed(ColorType color, IInnerPlayerControl? exceptBy = null)
{
return Players.Any(p => p.Character != null && p.Character != exceptBy && p.Character.PlayerInfo.CurrentOutfit.Color == color);
}
private ValueTask BroadcastJoinMessage(IMessageWriter message, bool clear, ClientPlayer player)
{
Message01JoinGameS2C.SerializeJoin(message, clear, Code, player, HostId);