From 87c3a853fdc937f7220bd93b55e377e67725a423 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 8 Feb 2012 19:11:12 +0100 Subject: [PATCH] Make market command unavailable to visitors, and set C_MOD flag The market command executes all trades that have become ready, by calling check_market() and check_trade(). This modifies game state, but market lacks C_MOD. That's wrong. But can it do harm? Turns out yes. check_trade() looks safe, but check_market() telexes seller and buyer while holding a copy of the commodity struct. If this telexes the player who sent the market command, and he has NF_INFORM on, the telegram notification may yield the processor. It then writes back its copy, triggering a generation oops. Any updates made by other threads meanwhile are wiped out, triggering a seqno mismatch oops. This can cause commodity trades to be executed multiple times, multiplying the sold commodities. Abuse seems tricky, but possible: conspiring trade partners trade commodities back and forth to multiply them. One of them needs to get the output backlog just right to make the telegram notification yield, and the timing right so that the MarketUpdate thread or another player's buy, market, reset, sell, set or trade command runs check_market() before his thread resumes. Closely related bug: visitors can trigger execution of trades by means of command market. That's clearly inappropriate. Broken in Empire 3. Reported by Scott C. Zielinski. --- src/lib/player/empmod.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/player/empmod.c b/src/lib/player/empmod.c index 058419c9..0022ef90 100644 --- a/src/lib/player/empmod.c +++ b/src/lib/player/empmod.c @@ -147,7 +147,7 @@ struct cmndstr player_coms[] = { 1, lload, C_MOD, NORM + CAP}, {"map [s|l|n|p|*|h]", 0, map, C_MOD, VIS}, {"march ", 1, march, C_MOD, NORM + CAP}, - {"market ", 0, mark, 0, VIS}, + {"market ", 0, mark, C_MOD, NORM}, {"mine ", 2, mine, C_MOD, NORM + MONEY + CAP}, {"mission []", 2, mission, C_MOD, NORM + CAP},