Multiple UBSAN errors (ipp.c, string.c)

Drawishe commented 4 months ago

Building a project with the gcc compiler with the -fsanitize=undefined option and running oss-fuzz targets on collected corpus resulted in some crashes.

Several crashes were found in the ipp.c file. Here they are:

ipp.c:2643:53: runtime error: left shift of 233 by 24 places cannot be represented in type 'int'
    #0 0x557c529d9f10 in ippReadIO /home/as/cups-oss/cups-opensource/cups/ipp.c:2643
    #1 0x557c529a73c7 in LoadIPP /home/as/cups-oss/cups-opensource/fuzzer/FuzzIPP.c:30
    #2 0x557c529a74cc in main /home/as/cups-oss/cups-opensource/fuzzer/FuzzIPP.c:66
    #3 0x7fa65de461c9 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #4 0x7fa65de46284 in __libc_start_main_impl ../csu/libc-start.c:360
    #5 0x557c529a72c0 in _start (/home/as/cups-oss/cups-opensource/fuzzer/FuzzIPP+0x5e2c0)

ipp.c:2681:35: runtime error: left shift of 235 by 24 places cannot be represented in type 'int'
    #0 0x55f47d1b313a in ippReadIO /home/as/cups-oss/cups-opensource/cups/ipp.c:2681
    #1 0x55f47d1803c7 in LoadIPP /home/as/cups-oss/cups-opensource/fuzzer/FuzzIPP.c:30
    #2 0x55f47d1804cc in main /home/as/cups-oss/cups-opensource/fuzzer/FuzzIPP.c:66
    #3 0x7fd4bf2461c9 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #4 0x7fd4bf246284 in __libc_start_main_impl ../csu/libc-start.c:360
    #5 0x55f47d1802c0 in _start (/home/as/cups-oss/cups-opensource/fuzzer/FuzzIPP+0x5e2c0)

ipp.c:2918:18: runtime error: left shift of 255 by 24 places cannot be represented in type 'int'
    #0 0x555c47735c60 in ippReadIO /home/as/cups-oss/cups-opensource/cups/ipp.c:2918
    #1 0x555c477023c7 in LoadIPP /home/as/cups-oss/cups-opensource/fuzzer/FuzzIPP.c:30
    #2 0x555c477024cc in main /home/as/cups-oss/cups-opensource/fuzzer/FuzzIPP.c:66
    #3 0x7f6e7be461c9 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #4 0x7f6e7be46284 in __libc_start_main_impl ../csu/libc-start.c:360
    #5 0x555c477022c0 in _start (/home/as/cups-oss/cups-opensource/fuzzer/FuzzIPP+0x5e2c0)

ipp.c:3014:54: runtime error: left shift of 255 by 24 places cannot be represented in type 'int'
    #0 0x559f1d1da234 in ippReadIO /home/as/cups-oss/cups-opensource/cups/ipp.c:3014
    #1 0x559f1d1a63c7 in LoadIPP /home/as/cups-oss/cups-opensource/fuzzer/FuzzIPP.c:30
    #2 0x559f1d1a64cc in main /home/as/cups-oss/cups-opensource/fuzzer/FuzzIPP.c:66
    #3 0x7f698c6461c9 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #4 0x7f698c646284 in __libc_start_main_impl ../csu/libc-start.c:360
    #5 0x559f1d1a62c0 in _start (/home/as/cups-oss/cups-opensource/fuzzer/FuzzIPP+0x5e2c0)

All of them arise from potential integer overflow due to bitwise operations - the result of shifting left char variable is assigned to int variable, which in some cases can cause integer overflow (if char variable, shifting left by 24 bits, has value >= 128).

Is this normal behaviour in cups, or is there some undefined behaviour? Do we need to cast it to an unsigned integer or some other type of variable?

Also, ubsan found some crashes in string.c file, which are associated with address missalignment:

string.c:1202:19: runtime error: member access within misaligned address 0x56366dabde11 for type 'struct _cups_sp_item_t', which requires 4 byte alignment
0x56366dabde11: note: pointer points here
 00 00 00  01 00 00 00 00 00 01 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00
              ^ 
    #0 0x56366bfd7a81 in compare_sp_items /home/as/cups-oss/cups-opensource/cups/string.c:1202
    #1 0x56366bfe6830 in cups_array_find /home/as/cups-oss/cups-opensource/cups/array.c:1205
    #2 0x56366bfe880a in cupsArrayFind /home/as/cups-oss/cups-opensource/cups/array.c:394
    #3 0x56366bfd9a9b in _cupsStrAlloc /home/as/cups-oss/cups-opensource/cups/string.c:545
    #4 0x56366bfc782b in ippReadIO /home/as/cups-oss/cups-opensource/cups/ipp.c:3095
    #5 0x56366bf933c7 in LoadIPP /home/as/cups-oss/cups-opensource/fuzzer/FuzzIPP.c:30
    #6 0x56366bf934cc in main /home/as/cups-oss/cups-opensource/fuzzer/FuzzIPP.c:66
    #7 0x7f9cb80461c9 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #8 0x7f9cb8046284 in __libc_start_main_impl ../csu/libc-start.c:360
    #9 0x56366bf932c0 in _start (/home/as/cups-oss/cups-opensource/fuzzer/FuzzIPP+0x5e2c0)

System information:

OS and its version: debian, 12
CUPS version: upstream (c220e78)

zdohnal commented 4 months ago

@michaelrsweet I'm not sure here, can you look into it?

michaelrsweet commented 4 months ago

In the future, please don't mix separate issues in a single bug.

Issue 1 (ipp.c): FALSE POSITIVE

This is not a valid security or crash concern. The code in question uses UNSIGNED characters with values from 0 to 255. Left shifting might result in bit 7 of the first byte resulting in a signed 32-bit integer result, but THAT IS INTENTIONAL since IPP integers are two's complement (signed) values.

Issue 2 (string.c): FALSE POSITIVE

The string pool makes use of one or two 32-bit integers at the beginning of the string (one is an optional guard and the other a reference counter). _cupsStrAlloc passes an offset string pointer to cupsArrayFind in order to determine whether the string is already in the pool. From the standpoint of the tools, this results in an "unaligned" pointer (more a pointer that is aligned to 32 bits instead of 64) but since we are comparing character strings the alignment requirement does not exist (char accesses are necessarily not aligned since you might need to access the Nth character of a string and not break things...)

OpenPrinting / cups

Multiple UBSAN errors (ipp.c, string.c) #899