The functions from <ctype.h> take an int as parameter. The value of this parameter must "either be representable as an unsigned char or be the value of the macro EOF". The functions addFlagsFromEnvVar and bzopen_or_bzdopen wrongly cast the argument to Int32, the correct type is UChar. After a direct cast to Int32, the argument may still be negative, which makes that cast wrong.
This issue has been reported downstream in the Perl sources, which include a sub-set of the bzip2 source. Believe that OpenBSD is a candidate for this issue causing a problem. Also, see https://wiki.sei.cmu.edu/confluence/display/c/STR37-C.+Arguments+to+character-handling+functions+must+be+representable+as+an+unsigned+char for details on a coding standard that mentions the isdigit issue.
So both cases the chars tested are user supplied and could in theory be signed chars. So casting to Int32 or int could create negative values. Which isspace and isdigit don't handle. Proposed patch: diff --git a/bzip2.c b/bzip2.c index 1538faf..9ef7536 100644 --- a/bzip2.c +++ b/bzip2.c @@ -1767,8 +1767,8 @@ void addFlagsFromEnvVar ( Cell** argList, Char* varName ) if (p[i] == 0) break; p += i; i = 0; - while (isspace((Int32)(p[0]))) p++; - while (p[i] != 0 && !isspace((Int32)(p[i]))) i++; + while (isspace((UChar)(p[0]))) p++; + while (p[i] != 0 && !isspace((UChar)(p[i]))) i++; if (i > 0) { k = i; if (k > FILE_NAME_LEN-10) k = FILE_NAME_LEN-10; for (j = 0; j < k; j++) tmpName[j] = p[j]; diff --git a/bzlib.c b/bzlib.c index 2178655..100873c 100644 --- a/bzlib.c +++ b/bzlib.c @@ -1408,7 +1408,7 @@ BZFILE * bzopen_or_bzdopen case 's': smallMode = 1; break; default: - if (isdigit((int)(*mode))) { + if (isdigit((unsigned char)(*mode))) { blockSize100k = *mode-BZ_HDR_0; } }
Em 2022-12-27 14:24, mark at klomp dot org via Bzip2-devel escreveu: > https://sourceware.org/bugzilla/show_bug.cgi?id=28283 > > Mark Wielaard <mark at klomp dot org> changed: > > What |Removed |Added > ---------------------------------------------------------------------------- > CC| |mark at klomp dot org > > --- Comment #2 from Mark Wielaard <mark at klomp dot org> --- > So both cases the chars tested are user supplied and could in theory be > signed > chars. So casting to Int32 or int could create negative values. Which > isspace > and isdigit don't handle. Proposed patch: > > diff --git a/bzip2.c b/bzip2.c > index 1538faf..9ef7536 100644 > --- a/bzip2.c > +++ b/bzip2.c > @@ -1767,8 +1767,8 @@ void addFlagsFromEnvVar ( Cell** argList, Char* > varName ) > if (p[i] == 0) break; > p += i; > i = 0; > - while (isspace((Int32)(p[0]))) p++; > - while (p[i] != 0 && !isspace((Int32)(p[i]))) i++; > + while (isspace((UChar)(p[0]))) p++; > + while (p[i] != 0 && !isspace((UChar)(p[i]))) i++; > if (i > 0) { > k = i; if (k > FILE_NAME_LEN-10) k = FILE_NAME_LEN-10; > for (j = 0; j < k; j++) tmpName[j] = p[j]; > diff --git a/bzlib.c b/bzlib.c > index 2178655..100873c 100644 > --- a/bzlib.c > +++ b/bzlib.c > @@ -1408,7 +1408,7 @@ BZFILE * bzopen_or_bzdopen > case 's': > smallMode = 1; break; > default: > - if (isdigit((int)(*mode))) { > + if (isdigit((unsigned char)(*mode))) { > blockSize100k = *mode-BZ_HDR_0; > } > } yes, I agree ric
commit fbc4b11da543753b3b803e5546f56e26ec90c2a7 Author: Mark Wielaard <mjw@redhat.com> Date: Tue Apr 9 21:11:02 2024 +0200 Make sure to call isdigit and isspace with unsigned char Casting to Int32 or int could create negative values. Which isspace and isdigit don't handle. SEI CERT C Coding Standard STR37-C. Resolve by casting to UChar or unsigned char instead of Int32 or int. https://sourceware.org/bugzilla/show_bug.cgi?id=28283