From ec5bcd1a0cbe2b197c9d797a41042400c2029c4a Mon Sep 17 00:00:00 2001
From: Andrew Deason <adeason@sinenomine.net>
Date: Wed, 2 Aug 2023 17:33:53 -0500
Subject: [PATCH] util: Avoid bad ascii[1] in volutil_GetPartitionID

If the caller gives "/vicep" or "vicep" to volutil_GetPartitionID(),
we'll strlcpy() an empty string into ascii[], setting ascii[0] to 0.
Then we check the value of ascii[1], which is uninitialized.

This doesn't result in any bad behavior, because we then immediately
check the value of ascii[0] (which is 0), and return -1. But reading
the uninitialized ascii[1] triggers errors in tools like valgrind, and
is possibly fragile for future code changes.

To avoid this, make sure ascii[] is initialized at the start of the
function, and check if we have copied an empty string into ascii[].

While we are here, also add tests in volutil-t for invalid "vicep*"
strings, to match the existing tests for invalid "/vicep*" strings.

Change-Id: I724f893d4bb6421b955c1c89629ab9f277be98bc
Reviewed-on: https://gerrit.openafs.org/15526
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
---
 src/util/volparse.c    | 7 +++++--
 tests/util/volutil-t.c | 8 +++++++-
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/src/util/volparse.c b/src/util/volparse.c
index 79004fe875..cd557052da 100644
--- a/src/util/volparse.c
+++ b/src/util/volparse.c
@@ -34,7 +34,7 @@ volutil_GetPartitionID(char *aname)
 {
     char tc;
     afs_int32 temp;
-    char ascii[3];
+    char ascii[3] = {0};
 
     tc = *aname;
     if (tc == 0)
@@ -53,7 +53,6 @@ volutil_GetPartitionID(char *aname)
 	    return temp;
     }
     /* otherwise check for vicepa or /vicepa, or just plain "a" */
-    ascii[2] = 0;
     if (strlen(aname) <= 2) {
 	strcpy(ascii, aname);
     } else if (!strncmp(aname, "/vicep", 6)) {
@@ -64,6 +63,10 @@ volutil_GetPartitionID(char *aname)
 	    return -1;  /* bad partition name: trailing characters */
     } else
 	return -1;		/* bad partition name */
+    if (ascii[0] == '\0') {
+	/* Invalid partition name "/vicep" or "vicep". */
+	return -1;
+    }
     /* now partitions are named /vicepa ... /vicepz, /vicepaa, /vicepab, .../vicepzz,
      * and are numbered from 0.  Do the appropriate conversion */
     if (ascii[1] == 0) {
diff --git a/tests/util/volutil-t.c b/tests/util/volutil-t.c
index c598071ab9..87f5b9c585 100644
--- a/tests/util/volutil-t.c
+++ b/tests/util/volutil-t.c
@@ -32,6 +32,8 @@
 char *out_of_range_names[] = {
     "/vicepiv",
     "/vicepzz",
+    "vicepiv",
+    "vicepzz",
     "iv",
     "zz",
     "255",
@@ -45,6 +47,10 @@ char *invalid_names[] = {
     "/vicep",
     "/vicepbogus",
     "/vicep0",
+    "vice",
+    "vicep",
+    "vicepbogus",
+    "vicep0",
     "foo",
     "!?",
     "-1",
@@ -173,7 +179,7 @@ test_partition_id_to_name(void)
 int
 main(int argc, char **argv)
 {
-    plan(1293);
+    plan(1299);
     test_partition_name_to_id();
     test_partition_id_to_name();
     return 0;