Avoid undefined behaviour with the <ctype.h> functions.

fix https://github.com/openssl/openssl/issues/25112

As defined in the C standard:

   In all cases the argument is an int, the value of which shall
   be representable as an unsigned char or shall equal the value
   of the macro EOF.  If the argument has any other value, the
   behavior is undefined.

This is because they're designed to work with the int values returned
by getc or fgetc; they need extra work to handle a char value.

If EOF is -1 (as it almost always is), with 8-bit bytes, the allowed
inputs to the ctype.h functions are:

   {-1, 0, 1, 2, 3, ..., 255}.

However, on platforms where char is signed, such as x86 with the
usual ABI, code like

   char *p = ...;
   ... isspace(*p) ...

may pass in values in the range:

   {-128, -127, -126, ..., -2, -1, 0, 1, ..., 127}.

This has two problems:

1. Inputs in the set {-128, -127, -126, ..., -2} are forbidden.

2. The non-EOF byte 0xff is conflated with the value EOF = -1, so
   even though the input is not forbidden, it may give the wrong
   answer.

Casting char inputs to unsigned char first works around this, by
mapping the (non-EOF character) range {-128, -127, ..., -1} to {128,
129, ..., 255}, leaving no collisions with EOF.  So the above
fragment needs to be:

   char *p = ...;
   ... isspace((unsigned char)*p) ...

This patch inserts unsigned char casts where necessary.  Most of the
cases I changed, I compile-tested using -Wchar-subscripts -Werror on
NetBSD, which defines the ctype.h functions as macros so that they
trigger the warning when the argument has type char.  The exceptions
are under #ifdef __VMS or #ifdef _WIN32.  I left alone calls where
the input is int where the cast would obviously be wrong; and I left
alone calls where the input is already unsigned char so the cast is
unnecessary.

Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
Reviewed-by: Paul Dale <ppzgs1@gmail.com>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/25113)

(cherry picked from commit 99548cd16e9dfd850a3958e417b9e02950f208f4)

diff --git a/crypto/riscvcap.c b/crypto/riscvcap.c
index 1cbfb4a574f35a6807b051037aaa4b52dd771b4a..d09f3063d362e7144034d2cfa4a9fc2a6561eaba 100644 (file)
--- a/crypto/riscvcap.c
+++ b/crypto/riscvcap.c
@@ -38,7 +38,7 @@ size_t OPENSSL_instrument_bus2(unsigned int *out, size_t cnt, size_t max)
 static void strtoupper(char *str)
 {
     for (char *x = str; *x; ++x)
-        *x = toupper(*x);
+        *x = toupper((unsigned char)*x);
 }
 
 /* parse_env() parses a RISC-V architecture string. An example of such a string

diff --git a/engines/e_loader_attic.c b/engines/e_loader_attic.c
index f6b87359d3631cd3d70fcd21cabf0ffafe257c47..4b138cb7e191b8f1e08e761adf2760da7d0fe522 100644 (file)
--- a/engines/e_loader_attic.c
+++ b/engines/e_loader_attic.c
@@ -988,7 +988,7 @@ static OSSL_STORE_LOADER_CTX *file_open_ex
 #ifdef _WIN32
         /* Windows file: URIs with a drive letter start with a / */
         if (p[0] == '/' && p[2] == ':' && p[3] == '/') {
-            char c = tolower(p[1]);
+            char c = tolower((unsigned char)p[1]);
 
             if (c >= 'a' && c <= 'z') {
                 p++;

diff --git a/providers/implementations/encode_decode/encode_key2text.c b/providers/implementations/encode_decode/encode_key2text.c
index 3e75a9afb370c697949b380146cf9bd11f6c4ad2..637fcf6a1214b32cb6613fe70a610842e2050857 100644 (file)
--- a/providers/implementations/encode_decode/encode_key2text.c
+++ b/providers/implementations/encode_decode/encode_key2text.c
@@ -112,7 +112,8 @@ static int print_labeled_bignum(BIO *out, const char *label, const BIGNUM *bn)
             use_sep = 0; /* The first byte on the next line doesnt have a : */
         }
         if (BIO_printf(out, "%s%c%c", use_sep ? ":" : "",
-                       tolower(p[0]), tolower(p[1])) <= 0)
+                       tolower((unsigned char)p[0]),
+                       tolower((unsigned char)p[1])) <= 0)
             goto err;
         ++bytes;
         p += 2;

diff --git a/providers/implementations/storemgmt/file_store.c b/providers/implementations/storemgmt/file_store.c
index bb8b2ab8625a1ef1a3dc42e0997852821326e7a3..3049a9c81133ccb234456d9fc87ca57018a3c522 100644 (file)
--- a/providers/implementations/storemgmt/file_store.c
+++ b/providers/implementations/storemgmt/file_store.c
@@ -238,7 +238,7 @@ static void *file_open(void *provctx, const char *uri)
 #ifdef _WIN32
         /* Windows file: URIs with a drive letter start with a / */
         if (p[0] == '/' && p[2] == ':' && p[3] == '/') {
-            char c = tolower(p[1]);
+            char c = tolower((unsigned char)p[1]);
 
             if (c >= 'a' && c <= 'z') {
                 p++;