summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorTaylor R Campbell <campbell+openssl@mumble.net>2023-03-29 07:48:44 +0200
committerTomas Mraz <tomas@openssl.org>2024-10-10 20:47:48 +0200
commit99548cd16e9dfd850a3958e417b9e02950f208f4 (patch)
tree138710bffdfe4a10309178120cb7aa86d2455254
parentRevert "TEMPORARY: run daily checks on PR" (diff)
downloadopenssl-99548cd16e9dfd850a3958e417b9e02950f208f4.tar.xz
openssl-99548cd16e9dfd850a3958e417b9e02950f208f4.zip
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)
-rw-r--r--crypto/riscvcap.c2
-rw-r--r--engines/e_loader_attic.c2
-rw-r--r--providers/implementations/encode_decode/encode_key2text.c3
-rw-r--r--providers/implementations/storemgmt/file_store.c2
4 files changed, 5 insertions, 4 deletions
diff --git a/crypto/riscvcap.c b/crypto/riscvcap.c
index 03e827fa37..b87fe4c94e 100644
--- a/crypto/riscvcap.c
+++ b/crypto/riscvcap.c
@@ -48,7 +48,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 84dff6e2c3..154f36cbdd 100644
--- a/engines/e_loader_attic.c
+++ b/engines/e_loader_attic.c
@@ -983,7 +983,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 c0c2923285..db0c8abe82 100644
--- 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 doesn't 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 32d03e611b..eea8f2a0f8 100644
--- a/providers/implementations/storemgmt/file_store.c
+++ b/providers/implementations/storemgmt/file_store.c
@@ -235,7 +235,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++;