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>
This commit is contained in:
Andrew Deason 2023-08-02 17:33:53 -05:00 committed by Benjamin Kaduk
parent 94c8a458a0
commit ec5bcd1a0c
2 changed files with 12 additions and 3 deletions

View File

@ -34,7 +34,7 @@ volutil_GetPartitionID(char *aname)
{ {
char tc; char tc;
afs_int32 temp; afs_int32 temp;
char ascii[3]; char ascii[3] = {0};
tc = *aname; tc = *aname;
if (tc == 0) if (tc == 0)
@ -53,7 +53,6 @@ volutil_GetPartitionID(char *aname)
return temp; return temp;
} }
/* otherwise check for vicepa or /vicepa, or just plain "a" */ /* otherwise check for vicepa or /vicepa, or just plain "a" */
ascii[2] = 0;
if (strlen(aname) <= 2) { if (strlen(aname) <= 2) {
strcpy(ascii, aname); strcpy(ascii, aname);
} else if (!strncmp(aname, "/vicep", 6)) { } else if (!strncmp(aname, "/vicep", 6)) {
@ -64,6 +63,10 @@ volutil_GetPartitionID(char *aname)
return -1; /* bad partition name: trailing characters */ return -1; /* bad partition name: trailing characters */
} else } else
return -1; /* bad partition name */ 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, /* now partitions are named /vicepa ... /vicepz, /vicepaa, /vicepab, .../vicepzz,
* and are numbered from 0. Do the appropriate conversion */ * and are numbered from 0. Do the appropriate conversion */
if (ascii[1] == 0) { if (ascii[1] == 0) {

View File

@ -32,6 +32,8 @@
char *out_of_range_names[] = { char *out_of_range_names[] = {
"/vicepiv", "/vicepiv",
"/vicepzz", "/vicepzz",
"vicepiv",
"vicepzz",
"iv", "iv",
"zz", "zz",
"255", "255",
@ -45,6 +47,10 @@ char *invalid_names[] = {
"/vicep", "/vicep",
"/vicepbogus", "/vicepbogus",
"/vicep0", "/vicep0",
"vice",
"vicep",
"vicepbogus",
"vicep0",
"foo", "foo",
"!?", "!?",
"-1", "-1",
@ -173,7 +179,7 @@ test_partition_id_to_name(void)
int int
main(int argc, char **argv) main(int argc, char **argv)
{ {
plan(1293); plan(1299);
test_partition_name_to_id(); test_partition_name_to_id();
test_partition_id_to_name(); test_partition_id_to_name();
return 0; return 0;