bomb drop fly paradrop recon sweep: Fix read before array

The code computing the length of the flight path checks whether the
path ends with 'h'.  When getpath() returns an empty path, it accesses
flightpath[-1].  This could set the length to -1 (unlikely), or crash
(even less likely).  The former could be abused to gain mobility for
sufficiently inefficient or short-ranged planes.  Found with valgrind.

Broken in commit 404a76f7, v4.3.27.

Historically, getpath() could return paths with or without 'h', and
the check was necessary.  It returned an empty path only when the
player gave no input, aborting the command.  When the player entered
the assembly point's coordinates, it returned "h".

Commit 404a76f7 accidentally changed it to return "" then.  Also broke
flying to the assembly point's coordinates.  Commit 0f1e14f (v4.3.31)
fixed that part by changing getpath()'s contract: always return paths
without 'h' ("" simply means empty path), and return NULL on invalid
input, including no input.

The flawed check is superfluous since then.  Drop it.

Signed-off-by: Markus Armbruster <armbru@pond.sub.org>

diff --git a/src/lib/commands/bomb.c b/src/lib/commands/bomb.c
index 39f1aa170815c182fb051f14868536380dc9aa06..6af2db6f8b1c35b3b1afbe5e17e72b4300c88c19 100644 (file)
--- a/src/lib/commands/bomb.c
+++ b/src/lib/commands/bomb.c
@@ -117,8 +117,6 @@ bomb(void)
     pr("target sector is %s\n", xyas(tx, ty, player->cnum));
     getsect(tx, ty, &target);
     ap_to_target = strlen(flightpath);
-    if (flightpath[ap_to_target - 1] == 'h')
-       ap_to_target--;
     pr("range to target is %d\n", ap_to_target);
     /*
      * select planes within range

diff --git a/src/lib/commands/drop.c b/src/lib/commands/drop.c
index ca39b4d489698ba2457df314ca600c33c10c5ac0..7fb72ddcc5ad03abb8d44c3cd0b5868c881da182 100644 (file)
--- a/src/lib/commands/drop.c
+++ b/src/lib/commands/drop.c
@@ -95,8 +95,6 @@ drop(void)
     }
 
     ap_to_target = strlen(flightpath);
-    if (flightpath[ap_to_target - 1] == 'h')
-       ap_to_target--;
     pr("range to target is %d\n", ap_to_target);
     /*
      * select planes within range

diff --git a/src/lib/commands/fly.c b/src/lib/commands/fly.c
index c82a7a4b2490dffccda80a9aa1ec81742a868964..d3218db55af56a4eb5fa5a5d9740085b07781546 100644 (file)
--- a/src/lib/commands/fly.c
+++ b/src/lib/commands/fly.c
@@ -92,8 +92,6 @@ fly(void)
     }
 
     ap_to_target = strlen(flightpath);
-    if (flightpath[ap_to_target - 1] == 'h')
-       ap_to_target--;
     pr("range to target is %d\n", ap_to_target);
     /*
      * select planes within range

diff --git a/src/lib/commands/para.c b/src/lib/commands/para.c
index 41a0d08adf343b9ef2af24b781a0e59fee9b02ba..d8665e9bd8823e9d93e7a535359a6782b581df7f 100644 (file)
--- a/src/lib/commands/para.c
+++ b/src/lib/commands/para.c
@@ -72,8 +72,6 @@ para(void)
     getsect(tx, ty, &target);
     pr("LZ is %s\n", xyas(tx, ty, player->cnum));
     ap_to_target = strlen(flightpath);
-    if (flightpath[ap_to_target - 1] == 'h')
-       ap_to_target--;
     pr("range to target is %d\n", ap_to_target);
     if (target.sct_own == player->cnum) {
        pr("You can't air-assault your own sector!\n");

diff --git a/src/lib/commands/reco.c b/src/lib/commands/reco.c
index 32f590c5cd758a8ab5cfccb70865291d077488a2..274f662d425b47ddf5b4323902672dc7fbb07ed4 100644 (file)
--- a/src/lib/commands/reco.c
+++ b/src/lib/commands/reco.c
@@ -75,8 +75,6 @@ reco(void)
     cno = target.gen.ef_type == EF_SHIP ? target.gen.uid : -1;
 
     ap_to_target = strlen(flightpath);
-    if (flightpath[ap_to_target - 1] == 'h')
-       ap_to_target--;
     pr("range to target is %d\n", ap_to_target);
     /*
      * select planes within range