Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SEGV in raster-interpret.c:1053 #1188

Closed
k-furman opened this issue Mar 5, 2025 · 2 comments · Fixed by #1189
Closed

SEGV in raster-interpret.c:1053 #1188

k-furman opened this issue Mar 5, 2025 · 2 comments · Fixed by #1189

Comments

@k-furman
Copy link
Contributor

k-furman commented Mar 5, 2025

Describe the bug
I have found a SEGV in scan_ps() function while fuzzing cups project with a harness from oss-fuzz.
When I called the _cupsRasterExecPS() function with the code argument consisting of a string like this "701#", I got a SIGSEGV when it was calling the scan_ps() function for the second time, specifically when it was dereferencing the *cur pointer in the for loop.
The problem is in the call to the strtol() function. We are trying to pass as base argument an int value recieved from atoi() func, which can be any int value, while strtol() is waiting for a value between 2 and 36 or 0.
Debugging this situation under gdb, i got these values before strtol() call:

1305              obj.value.number = strtol(cur + 1, &cur, atoi(start));
(gdb) info locals
obj = {type = CUPS_PS_NUMBER, value = {boolean = 0, name = '\000' <repeats 63 times>, number = 0, other = '\000' <repeats 63 times>, string = '\000' <repeats 63 times>}}
start = 0x502000000010 "701#"
cur = 0x502000000013 "#"
valptr = 0x0
valend = 0x7fffffffdd00 ""
parens = 32767

And these values after strtol() call:

(gdb) n
1306              break;
(gdb) info locals
obj = {type = CUPS_PS_NUMBER, value = {boolean = 0, name = '\000' <repeats 63 times>, number = 0, other = '\000' <repeats 63 times>, string = '\000' <repeats 63 times>}}
start = 0x502000000010 "701#"
cur = 0xaaaaaaaaaaaaaaaa <error: Cannot access memory at address 0xaaaaaaaaaaaaaaaa>
valptr = 0x0
valend = 0x7fffffffdd00 ""
parens = 32767

So, in this situation address of cur variable will be 0xaaaaaaaaaaaaaaaa, which is valid for if (!cur) check, but is not valid for if (!*cur). After this, *ptr address will be equal to cur address, which is not quite correct.

Idk how to fix it correctly, but i have some ideas for fixing it.
First idea is to check this value, and if it doesn't belong to 2..36 or 0, set base to 10:

diff --git a/cups/raster-interpret.c b/cups/raster-interpret.c
index 852a4db00..30be03118 100644
--- a/cups/raster-interpret.c
+++ b/cups/raster-interpret.c
@@ -1301,8 +1301,10 @@ scan_ps(_cups_ps_stack_t *st,            /* I  - Stack */
         /*
          * Integer with radix...
          */
-
-          obj.value.number = strtol(cur + 1, &cur, atoi(start));
+          if (atoi(start) >= 2 || atoi(start) <= 36 || atoi(start) == 0)
+            obj.value.number = strtol(cur + 1, &cur, atoi(start));
+          else
+            obj.value.number = strtol(cur + 1, &cur, 10);
          break;
        }
        else if (strchr(".Ee()<>[]{}/%", *cur) || isspace(*cur & 255))

Second idea - take the modulus of the atoi() value, but it should not be 1, so this will work:

diff --git a/cups/raster-interpret.c b/cups/raster-interpret.c
index 852a4db00..1e74d7848 100644
--- a/cups/raster-interpret.c
+++ b/cups/raster-interpret.c
@@ -1302,7 +1302,7 @@ scan_ps(_cups_ps_stack_t *st,             /* I  - Stack */
          * Integer with radix...
          */
 
-          obj.value.number = strtol(cur + 1, &cur, atoi(start));
+          obj.value.number = strtol(cur + 1, &cur, atoi(start) % 37 != 1 ? atoi(start) % 37 : 10);
          break;
        }
        else if (strchr(".Ee()<>[]{}/%", *cur) || isspace(*cur & 255))

Third idea - include errno.h library and extern errno variable, check errno for EINVAL and if it's true, set cur ptr to NULL:

diff --git a/cups/raster-interpret.c b/cups/raster-interpret.c
index 852a4db00..cd45f404e 100644
--- a/cups/raster-interpret.c
+++ b/cups/raster-interpret.c
@@ -12,7 +12,8 @@
 #include <cups/raster-private.h>
 #include <cups/ppd-private.h>
 #include "debug-internal.h"
-
+#include <errno.h>
+extern int errno;
 
 /*
  * Stack values for the PostScript mini-interpreter...
@@ -1303,6 +1304,8 @@ scan_ps(_cups_ps_stack_t *st,             /* I  - Stack */
          */
 
           obj.value.number = strtol(cur + 1, &cur, atoi(start));
+          if (errno == EINVAL) /* not in all c99 implementations - gcc OK */
+            cur = NULL;
          break;
        }
        else if (strchr(".Ee()<>[]{}/%", *cur) || isspace(*cur & 255))

To Reproduce
Steps to reproduce the behavior:

  1. Build cups with ASAN sanitizer:
export CC=clang
export CXX=clang++
export CFLAGS="-fsanitize=address -g -O0"
export CXXFLAGS="-fsanitize=address -g -O0"
./configure --enable-static --disable-shared
make -C cups/
  1. Get source code of fuzz_cups harness from oss-fuzz
  2. Build this harness:
clang -fsanitize=address,fuzzer -g -O0 -I./ -I./cups/ fuzz_cups.c -o fuzz_cups cups/libcups.a -lssl -lz -lavahi-common -lavahi-client -lcrypto
  1. Get an error input:
echo "701#" > crash
  1. Run ./fuzz_cups crash
  2. See ASAN report
AddressSanitizer:DEADLYSIGNAL
=================================================================
==3858056==ERROR: AddressSanitizer: SEGV on unknown address (pc 0x5555557789f0 bp 0x7fffffffd870 sp 0x7fffffffd180 T0)
==3858056==The signal is caused by a READ memory access.
==3858056==Hint: this fault was caused by a dereference of a high value address (see register values below).  Disassemble the provided pc to learn which register was used.
    #0 0x5555557789f0 in scan_ps /home/as/cups/cups/raster-interpret.c:1053:20
    #1 0x5555557782fd in _cupsRasterExecPS /home/as/cups/cups/raster-interpret.c:538:17
    #2 0x555555774a3d in LLVMFuzzerTestOneInput /home/as/cups/fuzz_cups.c:41:5
    #3 0x55555567b1b6 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) crtstuff.c
    #4 0x555555663def in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) crtstuff.c
    #5 0x555555669de1 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) crtstuff.c
    #6 0x555555695102 in main (/home/as/cups/fuzz_cups+0x141102) (BuildId: 3d3c68c374abffcdd8d23d5504f0bcc9dbd01f81)
    #7 0x7ffff7446249 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #8 0x7ffff7446304 in __libc_start_main csu/../csu/libc-start.c:360:3
    #9 0x55555565e760 in _start (/home/as/cups/fuzz_cups+0x10a760) (BuildId: 3d3c68c374abffcdd8d23d5504f0bcc9dbd01f81)

==3858056==Register values:
rax = 0x1555555555555555  rbx = 0x00007fffffffd180  rcx = 0xaaaaaaaaaaaaaaaa  rdx = 0x0000000000000000  
rdi = 0x0000000000000000  rsi = 0xffffffffffffffff  rbp = 0x00007fffffffd870  rsp = 0x00007fffffffd180  
 r8 = 0x00007fffffffff01   r9 = 0x0000000000000f01  r10 = 0xffffffffffffffff  r11 = 0x0000000000000000  
r12 = 0x0000502000000010  r13 = 0x0000555555891800  r14 = 0x0000502000000030  r15 = 0x0000000000000005  
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /home/as/cups/cups/raster-interpret.c:1053:20 in scan_ps
==3858056==ABORTING

System Information:

  • debian 12, clang-19.1.7
  • CUPS version upstream 745f21c
zdohnal added a commit to zdohnal/cups that referenced this issue Mar 6, 2025
Input for `atoi()` can be bad number for argument `base` in `strtol()`,
causing returning an incorrect pointer address and later segfault.

Break out from function if the base is incorrect.

Fixes OpenPrinting#1188
@zdohnal
Copy link
Member

zdohnal commented Mar 6, 2025

Hi @k-furman ,

I have created PR which fixes this, thank you so much for the analysis and the report!

@michaelrsweet
Copy link
Member

@zdohnal Don't forget to also apply any changes over in the libppd repository (I think that's the one that has inherited this code...)

zdohnal added a commit to zdohnal/cups that referenced this issue Mar 7, 2025
Input for `atoi()` can be bad number for argument `base` in `strtol()`,
causing returning an incorrect pointer address and later segfault.

Break out from function if the base is incorrect.

Fixes OpenPrinting#1188
zdohnal added a commit to zdohnal/cups that referenced this issue Mar 7, 2025
Input for `atoi()` can be bad number for argument `base` in `strtol()`,
causing returning an incorrect pointer address and later segfault.

Break out from function if the base is incorrect.

Fixes OpenPrinting#1188
zdohnal added a commit that referenced this issue Mar 10, 2025
Input for atoi() can be bad number for argument base in strtol(), causing returning an incorrect pointer address and later segfault.

Break out from function if the base is incorrect.

Fixes #1188
zdohnal added a commit that referenced this issue Mar 10, 2025
Input for atoi() can be bad number for argument base in strtol(), causing returning an incorrect pointer address and later segfault.

Break out from function if the base is incorrect.

Fixes #1188
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants