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>

diff --git a/src/lib/subs/lndsub.c b/src/lib/subs/lndsub.c
index 44ba35f4ca576bcdce9c51bb8f56a3c944b5e133..96bd53e12be8ea585dc23361ec0d12e73d5c5ead 100644 (file)
--- a/src/lib/subs/lndsub.c
+++ b/src/lib/subs/lndsub.c
@@ -1098,25 +1098,28 @@ lnd_mar_dir(struct emp_qelem *list, int dir, natid actor)
        }
     }
 
-    return 0;
+    return QEMPTY(list);
 }
 
 int
 lnd_mar_gauntlet(struct emp_qelem *list, int interdict, natid actor)
 {
     struct ulist *mlp = (struct ulist *)list->q_back;
-    coord newx = mlp->unit.land.lnd_x;
-    coord newy = mlp->unit.land.lnd_y;
+    coord newx, newy;
     int stopping, visible;
     struct emp_qelem *qp, *next;
     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);
     if (QEMPTY(list))
-       return stopping;
+       return 1;
     stopping |= lnd_check_mines(list);
     if (QEMPTY(list))
-       return stopping;
+       return 1;
 
     visible = 0;
     for (qp = list->q_back; qp != list; qp = next) {

diff --git a/src/lib/subs/shpsub.c b/src/lib/subs/shpsub.c
index 6905da00d8864bd9f045b67c0b89a61f1d76b514..7e61372ee9e417eaf209cb86d96abff4087c8ae3 100644 (file)
--- a/src/lib/subs/shpsub.c
+++ b/src/lib/subs/shpsub.c
@@ -876,23 +876,26 @@ shp_nav_dir(struct emp_qelem *list, int dir, natid actor)
        putship(mlp->unit.ship.shp_uid, &mlp->unit.ship);
     }
 
-    return 0;
+    return QEMPTY(list);
 }
 
 int
 shp_nav_gauntlet(struct emp_qelem *list, int interdict, natid actor)
 {
     struct ulist *mlp = (struct ulist *)list->q_back;
-    coord newx = mlp->unit.ship.shp_x;
-    coord newy = mlp->unit.ship.shp_y;
+    coord newx, newy;
     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);
     if (QEMPTY(list))
-       return stopping;
+       return 1;
     stopping |= shp_check_mines(list);
     if (QEMPTY(list))
-       return stopping;
+       return 1;
     if (interdict)
        stopping |= shp_interdict(list, newx, newy, actor);