stack-buffer-overflow in ppdCacheCreateWithPPD()

[IliaKash1]

Hello! I have found a stack-buffer-overflow in ppd-cache.c

I don't think it is exploitable in any way, so I am writing this to the issues tab, not the security tab.

It occurs on the line 1455

libppd/ppd/ppd-cache.c

Line 1455 in 45f67c5

char *newmax = (pwg_keyword[strlen(pwg_keyword) - 1] == 'n' ?

It is caused by the following order of events: ppd->custom_max is {inf, 86400}, inside pwgFormatSizeName() width becomes -2147483648, which leads to the function returning 0 (failure). That return value is never checked, meanwhile strlen(pwg_keyword) is equal to 0, which results in pwg_keyword[-1] on line 1455.

Sanitizer output:

==41466==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x75b4bb30003f at pc 0x5e8c9d056828 bp 0x7fff8c9f6970 sp 0x7fff8c9f6968
READ of size 1 at 0x75b4bb30003f thread T0
    #0 0x5e8c9d056827 in ppdCacheCreateWithPPD /orig/libppd/ppd/ppd-cache.c:1455:21
    #1 0x5e8c9d07437d in ppdLoadAttributes /orig/libppd/ppd/ppd-ipp.c:422:15
    #2 0x5e8c9d03d276 in main /orig/libppd/sbo_test.c:13:18
    #3 0x75b4be57e249 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #4 0x75b4be57e304 in __libc_start_main csu/../csu/libc-start.c:360:3
    #5 0x5e8c9cf5dae0 in _start (/orig/libppd/sbo_test+0x48ae0) (BuildId: 53cfdefa4074b60eef1c1155b6fd6b2bad6053dc)

Address 0x75b4bb30003f is located in stack of thread T0 at offset 63 in frame
    #0 0x5e8c9d054e0f in ppdCacheCreateWithPPD /orig/libppd/ppd/ppd-cache.c:1221

  This frame has 5 object(s):
    [32, 40) 'options' (line 1235)
    [64, 137) 'pwg_keyword' (line 1238) <== Memory access at offset 63 underflows this variable
    [176, 217) 'ppd_name' (line 1238)
    [256, 512) 'id' (line 1265)
    [576, 636) 'match_counts' (line 1597)
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow /orig/libppd/ppd/ppd-cache.c:1455:21 in ppdCacheCreateWithPPD
Shadow bytes around the buggy address:
  0x75b4bb2ffd80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x75b4bb2ffe00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x75b4bb2ffe80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x75b4bb2fff00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x75b4bb2fff80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x75b4bb300000: f1 f1 f1 f1 00 f2 f2[f2]00 00 00 00 00 00 00 00
  0x75b4bb300080: 00 01 f2 f2 f2 f2 00 00 00 00 00 01 f2 f2 f2 f2
  0x75b4bb300100: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x75b4bb300180: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x75b4bb300200: f2 f2 f2 f2 f2 f2 f2 f2 f8 f8 f8 f8 f8 f8 f8 f8
  0x75b4bb300280: f3 f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==41466==ABORTING

Steps to reproduce:

1. Build the project with sanitizers:

export CC=clang
export CXX=clang++

export CFLAGS="$CFLAGS -g -O0 -fsanitize=address "
export CXXFLAGS="$CXXFLAGS -g -O0 -fsanitize=address "
autoreconf -fi
./configure --enable-static --disable-shared --enable-mutool --with-mutool-path=/usr/bin/mutool
make

2. Build the example (perhaps should be done a bit differently, i just wanted to use .so files built with debug symbols, libcupsfilters as of 2f762327, libcups as of v2.4.15):

$CC $CFLAGS -O0 -o sbo_test sbo_test.c ./.libs/libppd.a -I./ppd ../libcupsfilters/.libs/libcupsfilters.so.2.0.0 ../cups/cups/libcups.so -Wl,-rpath,$(realpath ../libcupsfilters/.libs) -Wl,-rpath,$(realpath ../cups/cups) -lpdfio -lpng -lz -lwebp -lttf

sbo_test.c:

#include <stdint.h>
#include <stdlib.h>
#include <stdio.h>
#include <unistd.h>
#include <string.h>

#include "ppd.h"

int main(int argc, char **argv) {
  if (argc < 2) return 0;
  ppd_file_t *ppd = ppdOpenFile(argv[1]);
  if (!ppd) return 0;
  ipp_t *attrs = ppdLoadAttributes(ppd);
  if (attrs) {
    ippDelete(attrs);
  }

  ppdClose(ppd);
  return 0;
}

3. Launch the example with the provided input:

sbo_input.zip

./sbo_test sbo_input

Suggested workaround:

I don't really know if this is the correct approach, perhaps the cause of the overflow should be handled earlier (having inf as width is suspicious to me), but as a straightforward solution:

diff --git a/ppd/ppd-cache.c b/ppd/ppd-cache.c
index 01e98d8d..9f6b9449 100644
--- a/ppd/ppd-cache.c
+++ b/ppd/ppd-cache.c
@@ -1444,9 +1444,13 @@ ppdCacheCreateWithPPD(ppd_file_t *ppd)   // I - PPD file
     // Generate custom size data...
     //

-    pwgFormatSizeName(pwg_keyword, sizeof(pwg_keyword), "custom", "max",
-                     PWG_FROM_POINTS(ppd->custom_max[0]),
-                     PWG_FROM_POINTS(ppd->custom_max[1]), NULL);
+    if(!pwgFormatSizeName(pwg_keyword, sizeof(pwg_keyword), "custom", "max",
+                         PWG_FROM_POINTS(ppd->custom_max[0]),
+                         PWG_FROM_POINTS(ppd->custom_max[1]), NULL))
+    {
+      DEBUG_puts("ppdCacheCreateWithPPD: pwgFormatSizeName failure.");
+      goto create_error;
+    }

     // Some PPD files have upper limits too large to be treated with
     // int numbers, if we have an overflow (negative result for one