Fix likely misuse of ctype.h API #4397

ThalesBarretto · ThalesBarretto · commit 3b93a42702dd · 2025-02-28T16:10:01.000-03:00
The standard C &lt;ctype.h&gt; API takes type int as argument, but when
that argument value is neither (a) a value representable by type unsigned
char nor (b) the value of the macro EOF, the behaviour is undefined. See
C11, Sec. 7.4 Character handling &lt;ctype.h&gt; p.200, clause 1.

	The &lt;ctype.h&gt; functions are designed to take as arguments the
values returned by getc or fgetc, no for processing elements of an
arbitrary string stored in a char array. Safely processing arbitrary
strings requires explicit cast to unsigned char to keep the argument
values within the domain {EOF, 0, 1 ..., 255}.

	OTH, passing negative values {-128, -127, ..., -1} may trigger
undefined behaviour, and also the non-EOF 0xff can be conflated with -1,
which, although not forbidden, may give unintended results. Casting to
unsigned char avoids sign extension when implicitly converting to int.

	This commit introduces an explicit cast to unsigned char when
passing argument to the standard &lt;ctype.h&gt; function family.

Reported-by: riastradh
Signed-off-by: Thales Antunes de Oliveira Barretto &lt;thales.barretto.git@gmail.com&gt;
diff --git a/api/src/glfs.c b/api/src/glfs.c
@@ -850,7 +850,7 @@ pub_glfs_new(const char *volname)
     }
 
     for (i = 0; i < strlen(volname); i++) {
-        if (!isalnum(volname[i]) && (volname[i] != '_') &&
+        if (!isalnum((unsigned char)volname[i]) && (volname[i] != '_') &&
             (volname[i] != '-')) {
             errno = EINVAL;
             return NULL;
diff --git a/cli/src/cli-cmd-parser.c b/cli/src/cli-cmd-parser.c
@@ -562,7 +562,7 @@ cli_validate_volname(const char *volname)
     }
 
     for (i = 0; i < volname_len; i++) {
-        if (!isalnum(volname[i]) && (volname[i] != '_') &&
+        if (!isalnum((unsigned char)volname[i]) && (volname[i] != '_') &&
             (volname[i] != '-')) {
             cli_err(
                 "Volume name should not contain \"%c\""
@@ -1702,7 +1702,7 @@ gf_strip_whitespace(char *str, int len)
         return -1;
 
     for (i = 0; i < len; i++) {
-        if (!isspace(str[i]))
+        if (!isspace((unsigned char)str[i]))
             new_str[new_len++] = str[i];
     }
     new_str[new_len] = '\0';
@@ -3087,7 +3087,7 @@ gf_is_str_int(const char *value)
     fptr = str;
 
     while (*str) {
-        if (!isdigit(*str)) {
+        if (!isdigit((unsigned char)*str)) {
             flag = 1;
             goto out;
         }
@@ -4173,7 +4173,7 @@ cli_snap_clone_parse(dict_t *dict, const char **words, int wordcount)
     clonename = (char *)words[cmdi];
     for (i = 0; i < strlen(clonename); i++) {
         /* Following volume name convention */
-        if (!isalnum(clonename[i]) &&
+        if (!isalnum((unsigned char)clonename[i]) &&
             (clonename[i] != '_' && (clonename[i] != '-'))) {
             /* TODO : Is this message enough?? */
             cli_err(
@@ -4255,7 +4255,7 @@ cli_snap_create_parse(dict_t *dict, const char **words, int wordcount)
     snapname = (char *)words[cmdi];
     for (i = 0; i < strlen(snapname); i++) {
         /* Following volume name convention */
-        if (!isalnum(snapname[i]) &&
+        if (!isalnum((unsigned char)snapname[i]) &&
             (snapname[i] != '_' && (snapname[i] != '-'))) {
             /* TODO : Is this message enough?? */
             cli_err(
@@ -5468,7 +5468,7 @@ cli_cmd_validate_volume(char *volname)
     }
 
     for (i = 0; i < volname_len; i++)
-        if (!isalnum(volname[i]) && (volname[i] != '_') &&
+        if (!isalnum((unsigned char)volname[i]) && (volname[i] != '_') &&
             (volname[i] != '-')) {
             cli_err(
                 "Volume name should not contain \"%c\""
diff --git a/cli/src/cli-xml-output.c b/cli/src/cli-xml-output.c
@@ -3412,7 +3412,7 @@ _output_gsync_config(FILE *fp, xmlTextWriterPtr writer, char *op_name)
             break;
 
         v = resbuf + strlen(resbuf) - 1;
-        while (isspace(*v)) {
+        while (isspace((unsigned char)*v)) {
             /* strip trailing space */
             *v-- = '\0';
         }
@@ -3434,7 +3434,7 @@ _output_gsync_config(FILE *fp, xmlTextWriterPtr writer, char *op_name)
             goto out;
         }
         *v++ = '\0';
-        while (isspace(*v))
+        while (isspace((unsigned char)*v))
             v++;
         v = gf_strdup(v);
         if (!v) {
diff --git a/extras/benchmarking/rdd.c b/extras/benchmarking/rdd.c
@@ -209,7 +209,7 @@ string2bytesize(const char *str, unsigned long long *n)
     }
 
     for (s = str; *s != '\0'; s++) {
-        if (isspace(*s)) {
+        if (isspace((unsigned char)*s)) {
             continue;
         }
         if (*s == '-') {
diff --git a/extras/geo-rep/gsync-sync-gfid.c b/extras/geo-rep/gsync-sync-gfid.c
@@ -62,7 +62,7 @@ main(int argc, char *argv[])
 
         path += UUID_CANONICAL_FORM_LEN + 1;
 
-        while (isspace(*path))
+        while (isspace((unsigned char)*path))
             path++;
 
         len = strlen(line);
diff --git a/geo-replication/src/procdiggy.c b/geo-replication/src/procdiggy.c
@@ -55,7 +55,7 @@ pidinfo(pid_t pid, char **name)
         if (name && !*name) {
             p = strtail(buf, "Name:");
             if (p) {
-                while (isspace(*++p))
+                while (isspace((unsigned char)*++p))
                     ;
                 *name = gf_strdup(p);
                 if (!*name) {
@@ -71,7 +71,7 @@ pidinfo(pid_t pid, char **name)
             break;
     }
 
-    while (isspace(*++p))
+    while (isspace((unsigned char)*++p))
         ;
     ret = gf_string2int(p, &lpid);
     if (ret == -1)
diff --git a/libglusterfs/src/common-utils.c b/libglusterfs/src/common-utils.c
@@ -713,14 +713,14 @@ gf_trim(char *string)
         return NULL;
     }
 
-    for (s = string; isspace(*s); s++)
+    for (s = string; isspace((unsigned char)*s); s++)
         ;
 
     if (*s == 0)
         return s;
 
     t = s + strlen(s) - 1;
-    while (t > s && isspace(*t))
+    while (t > s && isspace((unsigned char)*t))
         t--;
     *++t = '\0';
 
@@ -778,7 +778,7 @@ gf_string2time(const char *str, time_t *n)
     }
 
     for (s = str; *s != '\0'; s++) {
-        if (isspace(*s))
+        if (isspace((unsigned char)*s))
             continue;
         if (*s == '-')
             return -1;
@@ -852,7 +852,7 @@ gf_string2percent(const char *str, double *n)
     }
 
     for (s = str; *s != '\0'; s++) {
-        if (isspace(*s))
+        if (isspace((unsigned char)*s))
             continue;
         if (*s == '-')
             return -1;
@@ -928,7 +928,7 @@ _gf_string2ulong(const char *str, unsigned long *n, int base)
     }
 
     for (s = str; *s != '\0'; s++) {
-        if (isspace(*s))
+        if (isspace((unsigned char)*s))
             continue;
         if (*s == '-')
             return -1;
@@ -971,7 +971,7 @@ _gf_string2uint(const char *str, unsigned int *n, int base)
     }
 
     for (s = str; *s != '\0'; s++) {
-        if (isspace(*s))
+        if (isspace((unsigned char)*s))
             continue;
         if (*s == '-')
             return -1;
@@ -1082,7 +1082,7 @@ _gf_string2ulonglong(const char *str, unsigned long long *n, int base)
     }
 
     for (s = str; *s != '\0'; s++) {
-        if (isspace(*s))
+        if (isspace((unsigned char)*s))
             continue;
         if (*s == '-')
             return -1;
@@ -1450,7 +1450,7 @@ gf_string2bytesize_range(const char *str, uint64_t *n, uint64_t umax)
     max = umax & 0x7fffffffffffffffLL;
 
     for (s = str; *s != '\0'; s++) {
-        if (isspace(*s))
+        if (isspace((unsigned char)*s))
             continue;
         if (*s == '-')
             return -1;
@@ -1549,7 +1549,7 @@ gf_string2percent_or_bytesize(const char *str, double *n,
     }
 
     for (s = str; *s != '\0'; s++) {
-        if (isspace(*s))
+        if (isspace((unsigned char)*s))
             continue;
         if (*s == '-')
             return -1;
@@ -1806,7 +1806,7 @@ strtail(char *str, const char *pattern)
 void
 skipwhite(char **s)
 {
-    while (isspace(**s))
+    while (isspace((unsigned char)**s))
         (*s)++;
 }
 
@@ -1995,7 +1995,8 @@ valid_host_name(char *address, int length)
         goto out;
     }
 
-    if (!isalnum(dup_addr[length - 1]) && (dup_addr[length - 1] != '*')) {
+    if (!isalnum((unsigned char)dup_addr[length - 1]) &&
+        (dup_addr[length - 1] != '*')) {
         ret = 0;
         goto out;
     }
@@ -2013,12 +2014,13 @@ valid_host_name(char *address, int length)
     do {
         str_len = strlen(temp_str);
 
-        if (!isalnum(temp_str[0]) || !isalnum(temp_str[str_len - 1])) {
+        if (!isalnum((unsigned char)temp_str[0]) ||
+            !isalnum((unsigned char)temp_str[str_len - 1])) {
             ret = 0;
             goto out;
         }
         for (i = 1; i < str_len; i++) {
-            if (!isalnum(temp_str[i]) && (temp_str[i] != '-')) {
+            if (!isalnum((unsigned char)temp_str[i]) && (temp_str[i] != '-')) {
                 ret = 0;
                 goto out;
             }
@@ -2049,7 +2051,8 @@ valid_ipv4_address(char *address, int length, gf_boolean_t wildcard_acc)
      * delimiters.
      */
     if (length <= 0 || (strstr(address, "..")) ||
-        (!isdigit(tmp[length - 1]) && (tmp[length - 1] != '*'))) {
+        (!isdigit((unsigned char)tmp[length - 1]) &&
+         (tmp[length - 1] != '*'))) {
         ret = 0;
         goto out;
     }
diff --git a/tests/basic/open-behind/tester.c b/tests/basic/open-behind/tester.c
@@ -82,7 +82,8 @@ buffer_get(context_t *ctx)
 static int32_t
 str_skip_spaces(context_t *ctx, int32_t current)
 {
-    while ((current > 0) && (current != '\n') && isspace(current)) {
+    while ((current > 0) && (current != '\n') &&
+           isspace((unsigned char)current)) {
         current = buffer_get(ctx);
     }
 
@@ -98,7 +99,7 @@ str_token(context_t *ctx, char *buffer, uint32_t size, int32_t current)
 
     len = 0;
     while ((size > 0) && (current > 0) && (current != '\n') &&
-           !isspace(current)) {
+           !isspace((unsigned char)current)) {
         len++;
         *buffer++ = current;
         size--;
diff --git a/xlators/cluster/ec/src/ec-code.c b/xlators/cluster/ec/src/ec-code.c
@@ -838,7 +838,7 @@ ec_code_proc_trim_left(char *text, ssize_t *length)
 {
     ssize_t len;
 
-    for (len = *length; (len > 0) && isspace(*text); len--) {
+    for (len = *length; (len > 0) && isspace((unsigned char)*text); len--) {
         text++;
     }
     *length = len;
@@ -856,7 +856,7 @@ ec_code_proc_trim_right(char *text, ssize_t *length, char sep)
 
     last = text;
     for (len = *length; (len > 0) && (*text != sep); len--) {
-        if (!isspace(*text)) {
+        if (!isspace((unsigned char)*text)) {
             last = text + 1;
         }
         text++;
diff --git a/xlators/debug/io-stats/src/io-stats.c b/xlators/debug/io-stats/src/io-stats.c
@@ -822,7 +822,7 @@ io_stats_dump_global_to_json_logfp(xlator_t *this,
     for (i = 0; i < GF_FOP_MAXVALUE; i++) {
         lc_fop_name = strdupa(gf_fop_list[i]);
         for (j = 0; lc_fop_name[j]; j++) {
-            lc_fop_name[j] = tolower(lc_fop_name[j]);
+            lc_fop_name[j] = tolower((unsigned char)lc_fop_name[j]);
         }
 
         fop_hits = GF_ATOMIC_GET(stats->fop_hits[i]);
@@ -879,7 +879,7 @@ io_stats_dump_global_to_json_logfp(xlator_t *this,
     for (i = 0; i < GF_UPCALL_FLAGS_MAXVALUE; i++) {
         lc_fop_name = strdupa(gf_upcall_list[i]);
         for (j = 0; lc_fop_name[j]; j++) {
-            lc_fop_name[j] = tolower(lc_fop_name[j]);
+            lc_fop_name[j] = tolower((unsigned char)lc_fop_name[j]);
         }
         fop_hits = GF_ATOMIC_GET(stats->upcall_hits[i]);
         if (interval == -1) {
diff --git a/xlators/mgmt/glusterd/src/glusterd-ganesha.c b/xlators/mgmt/glusterd/src/glusterd-ganesha.c
@@ -55,7 +55,7 @@ parsing_ganesha_ha_conf(const char *key)
         if (*pointer == '#') {
             continue;
         }
-        while (isblank(*pointer)) {
+        while (isblank((unsigned char)*pointer)) {
             pointer++;
         }
         if (strncmp(pointer, key, strlen(key))) {
@@ -83,7 +83,8 @@ parsing_ganesha_ha_conf(const char *key)
         do {
             end_pointer++;
         } while (!(*end_pointer == '\'' || *end_pointer == '"' ||
-                   isspace(*end_pointer) || *end_pointer == '\0'));
+                   isspace((unsigned char)*end_pointer) ||
+                   *end_pointer == '\0'));
         *end_pointer = '\0';
 
         /* got it. copy it and return */
diff --git a/xlators/mgmt/glusterd/src/glusterd-geo-rep.c b/xlators/mgmt/glusterd/src/glusterd-geo-rep.c
@@ -760,7 +760,7 @@ _fcbk_conftodict(char *resbuf, size_t blen, FILE *fp, void *data)
         if (!ptr)
             break;
         v = resbuf + strlen(resbuf) - 1;
-        while (isspace(*v))
+        while (isspace((unsigned char)*v))
             /* strip trailing space */
             *v-- = '\0';
         if (v == resbuf)
@@ -770,7 +770,7 @@ _fcbk_conftodict(char *resbuf, size_t blen, FILE *fp, void *data)
         if (!v)
             return -1;
         *v++ = '\0';
-        while (isspace(*v))
+        while (isspace((unsigned char)*v))
             v++;
         v = gf_strdup(v);
         if (!v)
@@ -826,7 +826,7 @@ _fcbk_statustostruct(char *resbuf, size_t blen, FILE *fp, void *data)
             break;
 
         v = resbuf + strlen(resbuf) - 1;
-        while (isspace(*v))
+        while (isspace((unsigned char)*v))
             /* strip trailing space */
             *v-- = '\0';
         if (v == resbuf)
@@ -836,7 +836,7 @@ _fcbk_statustostruct(char *resbuf, size_t blen, FILE *fp, void *data)
         if (!v)
             return -1;
         *v++ = '\0';
-        while (isspace(*v))
+        while (isspace((unsigned char)*v))
             v++;
         v = gf_strdup(v);
         if (!v)
@@ -4427,7 +4427,7 @@ glusterd_gsync_read_frm_status(char *path, char *buf, size_t blen)
             ret = -1;
         } else {
             char *p = buf + len - 1;
-            while (isspace(*p))
+            while (isspace((unsigned char)*p))
                 *p-- = '\0';
         }
     } else if (ret == 0)
diff --git a/xlators/mgmt/glusterd/src/glusterd-mountbroker.c b/xlators/mgmt/glusterd/src/glusterd-mountbroker.c
@@ -140,7 +140,7 @@ parse_mount_pattern_desc(gf_mount_spec_t *mspec, char *pdesc)
                 ret = SYNTAX_ERR;
                 goto out;
             }
-            while (!strchr("|&)", *c2) && !isspace(*c2))
+            while (!strchr("|&)", *c2) && !isspace((unsigned char)*c2))
                 c2++;
             skipwhite(&c2);
             switch (*c2) {
@@ -182,7 +182,7 @@ parse_mount_pattern_desc(gf_mount_spec_t *mspec, char *pdesc)
         c2 = ""; /* reset c2 */
         while (*c2 != ')') {
             c2 = curs;
-            while (!isspace(*c2) && *c2 != ')')
+            while (!isspace((unsigned char)*c2) && *c2 != ')')
                 c2++;
             sc = *c2;
             *c2 = '\0';
diff --git a/xlators/mgmt/glusterd/src/glusterd-pmap.c b/xlators/mgmt/glusterd/src/glusterd-pmap.c
@@ -150,7 +150,7 @@ pmap_registry_search(xlator_t *this, char *brickname, gf_boolean_t destroy)
     {
         brck = tmp_port->brickname;
         for (;;) {
-            for (i = 0; brck[i] && !isspace(brck[i]); ++i)
+            for (i = 0; brck[i] && !isspace((unsigned char)brck[i]); ++i)
                 ;
             if (i == 0 && brck[i] == '\0')
                 break;
@@ -175,7 +175,7 @@ pmap_registry_search(xlator_t *this, char *brickname, gf_boolean_t destroy)
              * Skip over *any* amount of whitespace, including
              * none (if we're already at the end of the string).
              */
-            while (isspace(*brck))
+            while (isspace((unsigned char)*brck))
                 ++brck;
             /*
              * We're either at the end of the string (which will be
diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c
diff --git a/xlators/mgmt/glusterd/src/glusterd-volume-set.c b/xlators/mgmt/glusterd/src/glusterd-volume-set.c
diff --git a/xlators/performance/md-cache/src/md-cache.c b/xlators/performance/md-cache/src/md-cache.c

Original file line number	Diff line number	Diff line change
`@@ -850,7 +850,7 @@ pub_glfs_new(const char *volname)`
`850`	`850`	`}`
`851`	`851`
`852`	`852`	`for (i = 0; i < strlen(volname); i++) {`
`853`		`- if (!isalnum(volname[i]) && (volname[i] != '_') &&`
	`853`	`+ if (!isalnum((unsigned char)volname[i]) && (volname[i] != '_') &&`
`854`	`854`	`(volname[i] != '-')) {`
`855`	`855`	`errno = EINVAL;`
`856`	`856`	`return NULL;`
Original file line number	Diff line number	Diff line change
`@@ -209,7 +209,7 @@ string2bytesize(const char str, unsigned long long n)`
`209`	`209`	`}`
`210`	`210`
`211`	`211`	`for (s = str; *s != '\0'; s++) {`
`212`		`- if (isspace(*s)) {`
	`212`	`+ if (isspace((unsigned char)*s)) {`
`213`	`213`	`continue;`
`214`	`214`	`}`
`215`	`215`	`if (*s == '-') {`
Original file line number	Diff line number	Diff line change
`@@ -55,7 +55,7 @@ pidinfo(pid_t pid, char **name)`
`55`	`55`	`if (name && !*name) {`
`56`	`56`	`p = strtail(buf, "Name:");`
`57`	`57`	`if (p) {`
`58`		`- while (isspace(*++p))`
	`58`	`+ while (isspace((unsigned char)*++p))`
`59`	`59`	`;`
`60`	`60`	`*name = gf_strdup(p);`
`61`	`61`	`if (!*name) {`
`@@ -71,7 +71,7 @@ pidinfo(pid_t pid, char **name)`
`71`	`71`	`break;`
`72`	`72`	`}`
`73`	`73`
`74`		`- while (isspace(*++p))`
	`74`	`+ while (isspace((unsigned char)*++p))`
`75`	`75`	`;`
`76`	`76`	`ret = gf_string2int(p, &lpid);`
`77`	`77`	`if (ret == -1)`
Original file line number	Diff line number	Diff line change
`@@ -838,7 +838,7 @@ ec_code_proc_trim_left(char text, ssize_t length)`
`838`	`838`	`{`
`839`	`839`	`ssize_t len;`
`840`	`840`
`841`		`- for (len = length; (len > 0) && isspace(text); len--) {`
	`841`	`+ for (len = length; (len > 0) && isspace((unsigned char)text); len--) {`
`842`	`842`	`text++;`
`843`	`843`	`}`
`844`	`844`	`*length = len;`
`@@ -856,7 +856,7 @@ ec_code_proc_trim_right(char text, ssize_t length, char sep)`
`856`	`856`
`857`	`857`	`last = text;`
`858`	`858`	`for (len = length; (len > 0) && (text != sep); len--) {`
`859`		`- if (!isspace(*text)) {`
	`859`	`+ if (!isspace((unsigned char)*text)) {`
`860`	`860`	`last = text + 1;`
`861`	`861`	`}`
`862`	`862`	`text++;`