From 40ec33b09958cd846bf5640ae990f9dd478a2135 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 22 Jan 2014 19:26:24 +0100 Subject: [PATCH] retreat: Reject invalid retreat paths Undocumented misfeature: retreat and lretreat accept anything as retreat path. The paths' actual consumers retreat_ship1() and retread_land1() silently ignore invalid direction characters. The retreat paths are in xdump, and invalid ones could conceivably confuse smart clients. Change the commands to reject invalid paths, and the consumers to oops on invalid direction characters. Note that invalid paths get rejected even when they're not actually used because the conditions argument contains a "c" for "cancel". Requiring the user give a new path so he can cancel the old one is comically bad design. Signed-off-by: Markus Armbruster --- src/lib/commands/retr.c | 12 ++++++++++-- src/lib/subs/retreat.c | 8 ++------ tests/retreat/01-retreat-1 | 6 ++++-- tests/retreat/final.xdump | 4 ++-- tests/retreat/journal.log | 32 ++++++++++++++++++++++---------- 5 files changed, 40 insertions(+), 22 deletions(-) diff --git a/src/lib/commands/retr.c b/src/lib/commands/retr.c index 9bf6f2396..562af9777 100644 --- a/src/lib/commands/retr.c +++ b/src/lib/commands/retr.c @@ -38,6 +38,7 @@ #include "commands.h" #include "empobj.h" #include "land.h" +#include "path.h" #include "retreat.h" #include "ship.h" @@ -84,9 +85,16 @@ retreat(int type) if (!snxtitem(&ni, type, player->argp[1], NULL)) return RET_SYN; nunits = 0; - if (player->argp[2] != NULL) + if (player->argp[2] != NULL) { pq = getstarg(player->argp[2], "Retreat path? ", buf1); - else + for (i = 0; i < RET_LEN - 1 && pq[i]; i++) { + if (chkdir(pq[i], DIR_STOP, DIR_LAST) < 0) { + pr("'%c' is not a valid direction...\n", pq[i]); + direrr(NULL, NULL, NULL); + return RET_SYN; + } + } + } else pq = NULL; rflags = 0; diff --git a/src/lib/subs/retreat.c b/src/lib/subs/retreat.c index 97e982aa0..d850aa0c1 100644 --- a/src/lib/subs/retreat.c +++ b/src/lib/subs/retreat.c @@ -202,9 +202,7 @@ retreat_ship1(struct shpstr *sp, char code, int orig) } dir = chkdir(sp->shp_rpath[0], DIR_STOP, DIR_LAST); memmove(sp->shp_rpath, sp->shp_rpath+1, sizeof(sp->shp_rpath) - 1); - if (dir < 0) - continue; - if (dir == DIR_STOP) + if (dir == DIR_STOP || CANT_HAPPEN(dir < 0)) break; dx = diroff[dir][0]; dy = diroff[dir][1]; @@ -386,9 +384,7 @@ retreat_land1(struct lndstr *lp, char code, int orig) } dir = chkdir(lp->lnd_rpath[0], DIR_STOP, DIR_LAST); memmove(lp->lnd_rpath, lp->lnd_rpath+1, sizeof(lp->lnd_rpath) - 1); - if (dir < 0) - continue; - if (dir == DIR_STOP) + if (dir == DIR_STOP || CANT_HAPPEN(dir < 0)) break; dx = diroff[dir][0]; dy = diroff[dir][1]; diff --git a/tests/retreat/01-retreat-1 b/tests/retreat/01-retreat-1 index ad55e03aa..8dae7de89 100644 --- a/tests/retreat/01-retreat-1 +++ b/tests/retreat/01-retreat-1 @@ -2,6 +2,7 @@ || retreat command | garbage path retr 0 garbage i +__cmd added 0 -1 0 | condition help, then junk conditions retr 1 g ? @@ -9,7 +10,7 @@ junk | long path, all conditions retr 2/3 bgyujnbgyujnbgyujn itshbdu | clear retreat order -retr 3 ignored c +retr 3 h c | group retreat order retr a ?uid=4 b h | show retreat orders @@ -18,6 +19,7 @@ retr || lretreat command | garbage path lret 0 garbage i +__cmd added 0 -1 0 | condition help, then junk conditions lret 1 g ? @@ -25,7 +27,7 @@ junk | long path, all conditions lret 2/3 yujnbgyujnbgyujnbg ihb | clear retreat order -lret 3 ignored c +lret 3 h c | group retreat order lret a ?uid=4 b h | show retreat orders diff --git a/tests/retreat/final.xdump b/tests/retreat/final.xdump index bef357167..e8fa5cf9f 100644 --- a/tests/retreat/final.xdump +++ b/tests/retreat/final.xdump @@ -147,7 +147,7 @@ owner xloc yloc des effic mobil off loyal terr0 terr1 terr2 terr3 dterr xdist yd /config config ship uid owner xloc yloc type effic mobil off tech opx opy mission radius fleet xstart xend ystart yend cargostart(0) cargostart(1) cargostart(2) cargostart(3) cargostart(4) cargostart(5) cargoend(0) cargoend(1) cargoend(2) cargoend(3) cargoend(4) cargoend(5) amtstart(0) amtstart(1) amtstart(2) amtstart(3) amtstart(4) amtstart(5) amtend(0) amtend(1) amtend(2) amtend(3) amtend(4) amtend(5) autonav civil milit shell gun petrol iron dust bar food oil lcm hcm uw rad pstage ptime access mquota path follow name xbuilt ybuilt builder rflags rpath -0 1 -3 1 2 100 127 0 20 0 0 none 0 "a" 0 0 0 0 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 10 0 0 0 0 0 0 0 0 0 0 0 0 healthy 0 0 0 "" 0 "" -3 1 1 (injured) "garbage" +0 1 -3 1 2 100 127 0 20 0 0 none 0 "a" 0 0 0 0 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 10 0 0 0 0 0 0 0 0 0 0 0 0 healthy 0 0 0 "" 0 "" -3 1 1 () "" 1 1 -3 1 2 100 127 0 20 0 0 none 0 "a" 0 0 0 0 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 10 0 0 0 0 0 0 0 0 0 0 0 0 healthy 0 0 0 "" 1 "" -3 1 1 () "" 2 1 -3 1 2 91 115 0 20 0 0 none 0 "" 0 0 0 0 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 9 0 0 0 0 0 0 0 0 0 0 0 0 healthy 0 0 0 "" 2 "" -3 1 1 (injured torped sonared helpless bombed depth-charged boarded) "bgyujnbgy" 3 1 -3 1 2 100 127 0 20 0 0 none 0 "a" 0 0 0 0 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 10 0 0 0 0 0 0 0 0 0 0 0 0 healthy 0 0 0 "" 3 "" -3 1 1 () "" @@ -353,7 +353,7 @@ uid owner xloc yloc type effic mobil off tech opx opy mission radius wing range /config config land uid owner xloc yloc type effic mobil off tech opx opy mission radius army ship harden retreat rflags rpath civil milit shell gun petrol iron dust bar food oil lcm hcm uw rad pstage ptime land access -0 1 -3 1 2 100 127 0 50 0 0 none 0 "a" -1 127 42 (injured) "garbage" 0 10 0 0 0 0 0 0 0 0 0 0 0 0 healthy 0 -1 0 +0 1 -3 1 2 100 127 0 50 0 0 none 0 "a" -1 127 42 () "" 0 10 0 0 0 0 0 0 0 0 0 0 0 0 healthy 0 -1 0 1 1 -3 1 2 100 127 0 50 0 0 none 0 "a" -1 127 42 () "" 0 10 0 0 0 0 0 0 0 0 0 0 0 0 healthy 0 -1 0 2 1 -3 1 2 89 113 0 50 0 0 none 0 "" -1 127 42 (injured helpless bombed) "yujnbgyuj" 0 9 0 0 0 0 0 0 0 0 0 0 0 0 healthy 0 -1 0 3 1 -3 1 2 100 127 0 50 0 0 none 0 "a" -1 127 42 () "" 0 10 0 0 0 0 0 0 0 0 0 0 0 0 healthy 0 -1 0 diff --git a/tests/retreat/journal.log b/tests/retreat/journal.log index 93a461544..ab6364031 100644 --- a/tests/retreat/journal.log +++ b/tests/retreat/journal.log @@ -22,9 +22,15 @@ Play#1 output Play#1 6 0 640 Play#1 input retr 0 garbage i Play#1 command retreat - Play#1 output Play#1 1 shp# ship type x,y fl path as flt? flags - Play#1 output Play#1 1 0 cs cargo ship -3,1 a garbage i - Play#1 output Play#1 1 1 ship + Play#1 output Play#1 1 'a' is not a valid direction... + Play#1 output Play#1 1 Legal directions are: + Play#1 output Play#1 1 y u + Play#1 output Play#1 1 g j + Play#1 output Play#1 1 b n + Play#1 output Play#1 1 Usage: retreat [i|t|s|h|b|d|u|c] + Play#1 output Play#1 6 0 640 + Play#1 input __cmd added 0 -1 0 + Play#1 command __cmd Play#1 output Play#1 6 0 639 Play#1 input retr 1 g Play#1 command retreat @@ -50,7 +56,7 @@ Play#1 output Play#1 1 3 cs cargo ship -3,1 a bgyujnbgy itshbdu Play#1 output Play#1 1 2 ships Play#1 output Play#1 6 0 638 - Play#1 input retr 3 ignored c + Play#1 input retr 3 h c Play#1 command retreat Play#1 output Play#1 1 shp# ship type x,y fl path as flt? flags Play#1 output Play#1 1 3 cs cargo ship -3,1 a @@ -67,7 +73,7 @@ Play#1 output Play#1 4 ship(s)? Play#1 input 0/1/2/3/4 Play#1 output Play#1 1 shp# ship type x,y fl path as flt? flags - Play#1 output Play#1 1 0 cs cargo ship -3,1 a garbage i + Play#1 output Play#1 1 0 cs cargo ship -3,1 a Play#1 output Play#1 1 1 cs cargo ship -3,1 a Play#1 output Play#1 1 2 cs cargo ship -3,1 bgyujnbgy itshbdu Play#1 output Play#1 1 3 cs cargo ship -3,1 a @@ -76,9 +82,15 @@ Play#1 output Play#1 6 0 635 Play#1 input lret 0 garbage i Play#1 command lretreat - Play#1 output Play#1 1 lnd# unit type x,y ar path as army? flags - Play#1 output Play#1 1 0 inf infantry -3,1 a garbage i - Play#1 output Play#1 1 1 unit + Play#1 output Play#1 1 'a' is not a valid direction... + Play#1 output Play#1 1 Legal directions are: + Play#1 output Play#1 1 y u + Play#1 output Play#1 1 g j + Play#1 output Play#1 1 b n + Play#1 output Play#1 1 Usage: lretreat [i|h|b|c] + Play#1 output Play#1 6 0 635 + Play#1 input __cmd added 0 -1 0 + Play#1 command __cmd Play#1 output Play#1 6 0 634 Play#1 input lret 1 g Play#1 command lretreat @@ -100,7 +112,7 @@ Play#1 output Play#1 1 3 inf infantry -3,1 a yujnbgyuj ihb Play#1 output Play#1 1 2 units Play#1 output Play#1 6 0 633 - Play#1 input lret 3 ignored c + Play#1 input lret 3 h c Play#1 command lretreat Play#1 output Play#1 1 lnd# unit type x,y ar path as army? flags Play#1 output Play#1 1 3 inf infantry -3,1 a @@ -117,7 +129,7 @@ Play#1 output Play#1 4 land(s)? Play#1 input 0/1/2/3/4 Play#1 output Play#1 1 lnd# unit type x,y ar path as army? flags - Play#1 output Play#1 1 0 inf infantry -3,1 a garbage i + Play#1 output Play#1 1 0 inf infantry -3,1 a Play#1 output Play#1 1 1 inf infantry -3,1 a Play#1 output Play#1 1 2 inf infantry -3,1 yujnbgyuj ihb Play#1 output Play#1 1 3 inf infantry -3,1 a -- 2.43.0