Fix unsafe use of shared buffers in commands drawing maps
authorMarkus Armbruster <armbru@pond.sub.org>
Thu, 25 Feb 2010 06:07:28 +0000 (07:07 +0100)
committerMarkus Armbruster <armbru@pond.sub.org>
Wed, 10 Mar 2010 08:44:27 +0000 (09:44 +0100)
Maps are generally drawn into static scratch buffers.  Each command
has its own buffers.

Static scratch buffers are safe as long as they're never used across
yields.  Player output can yield unless the command has flag C_MOD
set.  Commands lradar, path, radar, route, satellite, sect, survey
hadn't.  If such a command yields while using scratch buffers, another
instance of the command can clobber them.

Abuse seems tricky, but possible: if a malicious player stalls output
just right, a command yields while printing a map from the scratch
buffer.  It resumes only when the malicious player reads some output.
If another player runs the same command before that, it overwrites the
same static scratch buffer with its map.  The malicious player
receives the last such run's map.

4.2.8 fixed the same bug for bmap, lbmap, lmap, map, nmap, pbmap,
pmap, sbmap and smap.

All were broken in 4.2.0.  Except radar maps (lradar and radar) were
already broken in Empire 2 for AIX.
(cherry picked from commit 8bdb5c5c1b5a672a280664b798687adc158fe3f7)

src/lib/player/empmod.c

index fe75b408f2813c6001ca66ea8d6e6055cbf22d02..b9ec9d6a51dc8557e04e93bd29dea18129f5c95a 100644 (file)
@@ -141,7 +141,7 @@ struct cmndstr player_coms[] = {
      1, load, C_MOD, NORM + CAP},
     {"lookout <SHIPS>", 1, look, 0, NORM + CAP},
     {"lost", 0, lost, 0, NORM},
-    {"lradar <UNITS | SECTS>", 1, lrad, 0, NORM + CAP},
+    {"lradar <UNITS | SECTS>", 1, lrad, C_MOD, NORM + CAP},
     {"lretreat <UNITS|ARMY> <PATH> [i|h|b|c]",
      1, lretr, C_MOD, NORM + CAP},
     {"lstat <UNITS>", 0, lsta, C_MOD, NORM},
@@ -176,7 +176,7 @@ struct cmndstr player_coms[] = {
     {"origin <SECT|COUNTRY|~>", 1, orig, C_MOD, NORM},
     {"paradrop <cargo-PLANES> <fighter-PLANES> <ap-SECT> <PATH|DESTINATION>",
      3, para, C_MOD, NORM + MONEY + CAP},
-    {"path <SECT>", 0, path, 0, NORM},
+    {"path <SECT>", 0, path, C_MOD, NORM},
     {"payoff <SHIPS>", 0, payo, C_MOD, NORM},
     {"pbmap <SECTS|PLANE> [s|l|n|p|r|t|*|h]", 0, map, C_MOD, NORM},
     {"pboard <PLANES>", 3, pboa, C_MOD, NORM + MONEY + CAP},
@@ -192,7 +192,7 @@ struct cmndstr player_coms[] = {
     {"pstat <PLANES>", 0, pstat, 0, NORM},
     {"qorder <SHIPS>", 0, qorde, C_MOD, NORM + CAP},
     {"quit", 0, quit, 0, VIS},
-    {"radar <SHIPS | SECTS>", 1, rada, 0, NORM + CAP},
+    {"radar <SHIPS | SECTS>", 1, rada, C_MOD, NORM + CAP},
     {"range <PLANES> <range>", 1, range, C_MOD, NORM + CAP},
     {"read [yes|no|<CNUM/CNAME>]", 0, rea, C_MOD, VIS},
     {"realm <number> [<SECTS>]", 0, real, C_MOD, NORM},
@@ -208,17 +208,17 @@ struct cmndstr player_coms[] = {
     {"resource <SECTS>", 0, reso, 0, NORM},
     {"retreat <SHIPS|FLEET> <PATH> [i|t|s|h|b|d|u|c]",
      1, retr, C_MOD, NORM + CAP},
-    {"route <COMM> <SECTS>", 1, rout, 0, NORM},
+    {"route <COMM> <SECTS>", 1, rout, C_MOD, NORM},
     {"sabotage <UNITS>", 1, sabo, C_MOD, NORM + MONEY + CAP},
     {"sail <SHIPS> <PATH>", 1, sail, C_MOD, NORM + CAP},
     {"satellite <PLANE> [<se|sh|l> [?cond&cond&...]]",
-     1, sate, 0, NORM + MONEY + CAP},
+     1, sate, C_MOD, NORM + MONEY + CAP},
     {"sbmap <SECTS|SHIP> [s|l|n|p|r|t|*|h]", 0, map, C_MOD, NORM},
     {"scrap <s|p|l> <SHIPS|PLANES|UNITS>",
      2, scra, C_MOD, NORM + MONEY + CAP},
     {"scuttle <s|p|l> <SHIPS|PLANES|UNITS>", 5, scut, C_MOD, NORM + CAP},
     {"sdump <SHIPS> [<fields>]", 0, sdump, 0, NORM},
-    {"sect <SECTS>", 0, sct, 0, NORM},
+    {"sect <SECTS>", 0, sct, C_MOD, NORM},
     {"sell <COMM> <SECTS> <NUM> <NUM>", 1, sell, C_MOD, NORM + CAP},
     {"set <TYPE> <SHIPS|PLANES|UNITS|NUKES> <PRICE>",
      1, set, C_MOD, NORM + CAP},
@@ -245,7 +245,7 @@ struct cmndstr player_coms[] = {
     {"stop <TYPE> <SECTS|PLANES|SHIPS|UNITS>", 1, stop, C_MOD, NORM + CAP},
     {"strength <SECTS>", 1, stre, C_MOD, NORM},
     {"supply <LAND UNITS>", 1, supp, C_MOD, NORM + CAP},
-    {"survey <SELECTOR> <SECTS>", 0, surv, 0, NORM + CAP},
+    {"survey <SELECTOR> <SECTS>", 0, surv, C_MOD, NORM + CAP},
     {"swapsector <SECT> <SECT>", 0, swaps, C_MOD, GOD},
     {"sweep <sweep-PLANES> <fighter-PLANES> <ap-SECT> <PATH|DESTINATION>",
      3, reco, C_MOD, NORM + MONEY + CAP},