navigate march retreat lretreat: Fix read beyond buffer

shp_nav_gauntlet() and lnd_mar_gauntlet() read beyond the list head
when the list is empty.  The values read aren't used then.  Could
conceivably crash the server anyway, but it's unlikely.

Empty list happens when shp_nav_dir(), lnd_mar_dir() empty the list
and return zero.  Broken in commit beedf8d, v4.3.33.  Occurs in
navi-march-test (since the last commit) and in retreat-test.

Change shp_nav_dir() and lnd_mar_dir() to return one then.  For
additional safety, make shp_nav_gauntlet() and lnd_mar_gauntlet() oops
on empty list and recover safely.

I think I originally found this bug with -fsanitize, but I've since
upgraded, and I can't diagnose it that way anymore.

Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
This commit is contained in:
Markus Armbruster 2015-09-20 13:17:33 +02:00
parent 493dc5f941
commit 25c7d3798b
2 changed files with 16 additions and 10 deletions

View file

@ -1098,25 +1098,28 @@ lnd_mar_dir(struct emp_qelem *list, int dir, natid actor)
} }
} }
return 0; return QEMPTY(list);
} }
int int
lnd_mar_gauntlet(struct emp_qelem *list, int interdict, natid actor) lnd_mar_gauntlet(struct emp_qelem *list, int interdict, natid actor)
{ {
struct ulist *mlp = (struct ulist *)list->q_back; struct ulist *mlp = (struct ulist *)list->q_back;
coord newx = mlp->unit.land.lnd_x; coord newx, newy;
coord newy = mlp->unit.land.lnd_y;
int stopping, visible; int stopping, visible;
struct emp_qelem *qp, *next; struct emp_qelem *qp, *next;
struct ulist *llp; struct ulist *llp;
if (CANT_HAPPEN(QEMPTY(list)))
return 1;
newx = mlp->unit.land.lnd_x;
newy = mlp->unit.land.lnd_y;
stopping = lnd_sweep(list, 0, 1, actor); stopping = lnd_sweep(list, 0, 1, actor);
if (QEMPTY(list)) if (QEMPTY(list))
return stopping; return 1;
stopping |= lnd_check_mines(list); stopping |= lnd_check_mines(list);
if (QEMPTY(list)) if (QEMPTY(list))
return stopping; return 1;
visible = 0; visible = 0;
for (qp = list->q_back; qp != list; qp = next) { for (qp = list->q_back; qp != list; qp = next) {

View file

@ -876,23 +876,26 @@ shp_nav_dir(struct emp_qelem *list, int dir, natid actor)
putship(mlp->unit.ship.shp_uid, &mlp->unit.ship); putship(mlp->unit.ship.shp_uid, &mlp->unit.ship);
} }
return 0; return QEMPTY(list);
} }
int int
shp_nav_gauntlet(struct emp_qelem *list, int interdict, natid actor) shp_nav_gauntlet(struct emp_qelem *list, int interdict, natid actor)
{ {
struct ulist *mlp = (struct ulist *)list->q_back; struct ulist *mlp = (struct ulist *)list->q_back;
coord newx = mlp->unit.ship.shp_x; coord newx, newy;
coord newy = mlp->unit.ship.shp_y;
int stopping; int stopping;
if (CANT_HAPPEN(QEMPTY(list)))
return 1;
newx = mlp->unit.ship.shp_x;
newy = mlp->unit.ship.shp_y;
stopping = shp_sweep(list, 0, 0, actor); stopping = shp_sweep(list, 0, 0, actor);
if (QEMPTY(list)) if (QEMPTY(list))
return stopping; return 1;
stopping |= shp_check_mines(list); stopping |= shp_check_mines(list);
if (QEMPTY(list)) if (QEMPTY(list))
return stopping; return 1;
if (interdict) if (interdict)
stopping |= shp_interdict(list, newx, newy, actor); stopping |= shp_interdict(list, newx, newy, actor);