jeirmeister/Cockatrice
1
0
Fork 0
mirror of https://github.com/Cockatrice/Cockatrice.git synced 2026-09-28 08:52:19 -07:00
Code Issues Projects Releases Packages Wiki Activity Actions 5

Extract evaluateCreateCounter helper to directly test reserved-name guard in cmdCreateCounter

Browse source
This commit is contained in:
DawnFire42 2026-08-10 16:27:13 -04:00
parent 1f5df5b0e1
commit 9121064709
No known key found for this signature in database
GPG key ID: 24BB855EE2911B33
3 changed files with 52 additions and 14 deletions
Download patch file Download diff file Expand all files Collapse all files

20
libcockatrice_network/libcockatrice/network/server/remote/game/server_player.cpp
View file

@ -495,22 +495,32 @@ Server_Player::cmdIncCounter(const Command_IncCounter &cmd, ResponseContainer &
} }
Response::ResponseCode Response::ResponseCode
Server_Player::cmdCreateCounter(const Command_CreateCounter &cmd, ResponseContainer & /*rc*/, GameEventStorage &ges) Server_Player::evaluateCreateCounter(bool gameStarted, bool playerConceded, const QString &counterName)
{ {
if (!game->getGameStarted()) { if (!gameStarted) {
return Response::RespGameNotStarted; return Response::RespGameNotStarted;
} }
if (conceded) { if (playerConceded) {
return Response::RespContextError; return Response::RespContextError;
} }
const QString counterName = nameFromStdString(cmd.counter_name());
// Reserved system counter names (commander/partner tax) are how clients identify // Reserved system counter names (commander/partner tax) are how clients identify
// server-managed tax counters for rendering and logging; a client must not be able // server-managed tax counters for rendering and logging; a client must not be able
// to spoof one via a user-created counter. // to spoof one via a user-created counter.
if (CounterNames::isTaxCounter(counterName)) { if (CounterNames::isTaxCounter(counterName)) {
return Response::RespFunctionNotAllowed; return Response::RespFunctionNotAllowed;
} }
return Response::RespOk;
}
Response::ResponseCode
Server_Player::cmdCreateCounter(const Command_CreateCounter &cmd, ResponseContainer & /*rc*/, GameEventStorage &ges)
{
const QString counterName = nameFromStdString(cmd.counter_name());
const Response::ResponseCode authResult = evaluateCreateCounter(game->getGameStarted(), conceded, counterName);
if (authResult != Response::RespOk) {
return authResult;
}
auto *c = new Server_Counter(newCounterId(), counterName, cmd.counter_color(), cmd.radius(), cmd.value()); auto *c = new Server_Counter(newCounterId(), counterName, cmd.counter_color(), cmd.radius(), cmd.value());
addCounter(c); addCounter(c);

14
libcockatrice_network/libcockatrice/network/server/remote/game/server_player.h
View file

@ -86,6 +86,20 @@ public:
int counterId, int counterId,
const Server_Counter *counter); const Server_Counter *counter);
/**
* @brief Decide whether a client may create a counter with the given name.
*
* Reserved system counter names (commander/partner tax) are rejected to prevent
* clients from spoofing server-managed tax counters.
*
* @param gameStarted Whether the game has started
* @param playerConceded Whether the player has conceded
* @param counterName Name requested for the new counter
* @return Response::RespOk if permitted, otherwise the error response for the client.
*/
static Response::ResponseCode
evaluateCreateCounter(bool gameStarted, bool playerConceded, const QString &counterName);
/** @} */ /** @} */
void setupZones() override; void setupZones() override;

32
tests/command_zone_tests/counter_command_auth_test.cpp
View file

@ -150,23 +150,37 @@ TEST(EvaluateSetCounterActive, RejectsDisablingPartnerTaxWhenAccumulated)
Response::RespContextError); Response::RespContextError);
} }
// CounterNames::isTaxCounter (guards cmdCreateCounter against reserved names) // evaluateCreateCounter
TEST(CounterNamesIsTaxCounter, RejectsCommanderTaxName) TEST(EvaluateCreateCounter, RejectsWhenGameNotStarted)
{ {
EXPECT_TRUE(CounterNames::isTaxCounter(CounterNames::CommanderTax)); EXPECT_EQ(Server_Player::evaluateCreateCounter(/*gameStarted=*/false, /*playerConceded=*/false, "mycounter"),
Response::RespGameNotStarted);
} }
TEST(CounterNamesIsTaxCounter, RejectsPartnerTaxName) TEST(EvaluateCreateCounter, RejectsWhenPlayerConceded)
{ {
EXPECT_TRUE(CounterNames::isTaxCounter(CounterNames::PartnerTax)); EXPECT_EQ(Server_Player::evaluateCreateCounter(/*gameStarted=*/true, /*playerConceded=*/true, "mycounter"),
Response::RespContextError);
} }
TEST(CounterNamesIsTaxCounter, AllowsOrdinaryName) TEST(EvaluateCreateCounter, RejectsCommanderTaxName)
{ {
EXPECT_FALSE(CounterNames::isTaxCounter("life")); EXPECT_EQ(Server_Player::evaluateCreateCounter(true, false, CounterNames::CommanderTax),
EXPECT_FALSE(CounterNames::isTaxCounter("poison")); Response::RespFunctionNotAllowed);
EXPECT_FALSE(CounterNames::isTaxCounter("")); }
TEST(EvaluateCreateCounter, RejectsPartnerTaxName)
{
EXPECT_EQ(Server_Player::evaluateCreateCounter(true, false, CounterNames::PartnerTax),
Response::RespFunctionNotAllowed);
}
TEST(EvaluateCreateCounter, AllowsOrdinaryNames)
{
EXPECT_EQ(Server_Player::evaluateCreateCounter(true, false, "life"), Response::RespOk);
EXPECT_EQ(Server_Player::evaluateCreateCounter(true, false, "poison"), Response::RespOk);
EXPECT_EQ(Server_Player::evaluateCreateCounter(true, false, ""), Response::RespOk);
} }
// evaluateModifyCounter (shared by cmdIncCounter / cmdSetCounter) // evaluateModifyCounter (shared by cmdIncCounter / cmdSetCounter)