[Bug bzip2/28283] New: undefined behavior for isdigit and isspace

[Bug bzip2/28283] New: undefined behavior for isdigit and isspace

[roland.illig at gmx dot de]

https://sourceware.org/bugzilla/show_bug.cgi?id=28283

            Bug ID: 28283
           Summary: undefined behavior for isdigit and isspace
           Product: bzip2
           Version: unspecified
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: bzip2
          Assignee: nobody at sourceware dot org
          Reporter: roland.illig at gmx dot de
                CC: bzip2-devel at sourceware dot org
  Target Milestone: ---

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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug bzip2/28283] undefined behavior for isdigit and isspace

[pmqs at outlook dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28283

Paul Marquess <pmqs at outlook dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |pmqs at outlook dot com

--- Comment #1 from Paul Marquess <pmqs at outlook dot com> ---
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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug bzip2/28283] undefined behavior for isdigit and isspace

[mark at klomp dot org]

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;
          }
       }

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug bzip2/28283] undefined behavior for isdigit and isspace

[email at arsoftware dot net.br]

https://sourceware.org/bugzilla/show_bug.cgi?id=28283

--- Comment #3 from email at arsoftware dot net.br ---
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

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug bzip2/28283] undefined behavior for isdigit and isspace

[mark at klomp dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28283

Mark Wielaard <mark at klomp dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |FIXED

--- Comment #4 from Mark Wielaard <mark at klomp dot org> ---
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

-- 
You are receiving this mail because:
You are on the CC list for the bug.