search

OpenPrinting / libppd

Apache License 2.0
2 stars 16 forks source link

Fix buffer overflow in ppdLoadAttributes(). #51

Closed dcoppa closed 3 weeks ago

dcoppa commented 3 weeks ago

On my musl libc based Linux distribution (Chimera Linux), all my print jobs were failing:

D [06/Nov/2024:07:34:56 +0100] [Job 18] Applying default options... D [06/Nov/2024:07:34:56 +0100] [Job 18] Adding start banner page "none". D [06/Nov/2024:07:34:56 +0100] [Job 18] Queued on "ENVY_4520" by "dcoppa". D [06/Nov/2024:07:34:56 +0100] [Job 18] Auto-typing file... D [06/Nov/2024:07:34:56 +0100] [Job 18] Request file type is text/plain. D [06/Nov/2024:07:34:56 +0100] [Job 18] File of type text/plain queued by "dcoppa". D [06/Nov/2024:07:34:56 +0100] [Job 18] Adding end banner page "none". D [06/Nov/2024:07:34:56 +0100] [Job 18] time-at-processing=1730874896 D [06/Nov/2024:07:34:56 +0100] [Job 18] 3 filters for job: D [06/Nov/2024:07:34:56 +0100] [Job 18] - (text/plain to application/vnd.universal-input, cost 0) D [06/Nov/2024:07:34:56 +0100] [Job 18] universal (application/vnd.universal-input to application/vnd.cups-raster, cost 0) D [06/Nov/2024:07:34:56 +0100] [Job 18] hpcups (application/vnd.cups-raster to printer/ENVY_4520, cost 0) D [06/Nov/2024:07:34:56 +0100] [Job 18] job-sheets=none,none D [06/Nov/2024:07:34:56 +0100] [Job 18] Mapping media to Pagesize=A4 D [06/Nov/2024:07:34:56 +0100] [Job 18] After mapping finishings PageSize=A4 D [06/Nov/2024:07:34:56 +0100] [Job 18] argv[0]="ENVY_4520" D [06/Nov/2024:07:34:56 +0100] [Job 18] argv[1]="18" D [06/Nov/2024:07:34:56 +0100] [Job 18] argv[2]="dcoppa" D [06/Nov/2024:07:34:56 +0100] [Job 18] argv[3]="trial.txt" D [06/Nov/2024:07:34:56 +0100] [Job 18] argv[4]="1" D [06/Nov/2024:07:34:56 +0100] [Job 18] argv[5]="finishings=3 media=a4 number-up=1 print-color-mode=color job-uuid=urn:uuid:9b22db21-acf4-3780-6912-ecac5aa9da16 job-originating-host-name=localhost date-time-at-creation= date-time-at-processing= time-at-creation=1730874896 time-at-processing=1730874896 document-name-supplied=trial.txt PageSize=A4" D [06/Nov/2024:07:34:56 +0100] [Job 18] argv[6]="/var/spool/cups/d00018-001" D [06/Nov/2024:07:34:56 +0100] [Job 18] envp[0]="CUPS_CACHEDIR=/var/cache/cups" D [06/Nov/2024:07:34:56 +0100] [Job 18] envp[1]="CUPS_DATADIR=/usr/share/cups" D [06/Nov/2024:07:34:56 +0100] [Job 18] envp[2]="CUPS_DOCROOT=/usr/share/cups/doc" D [06/Nov/2024:07:34:56 +0100] [Job 18] envp[3]="CUPS_REQUESTROOT=/var/spool/cups" D [06/Nov/2024:07:34:56 +0100] [Job 18] envp[4]="CUPS_SERVERBIN=/usr/lib/cups" D [06/Nov/2024:07:34:56 +0100] [Job 18] envp[5]="CUPS_SERVERROOT=/etc/cups" D [06/Nov/2024:07:34:56 +0100] [Job 18] envp[6]="CUPS_STATEDIR=/run/cups" D [06/Nov/2024:07:34:56 +0100] [Job 18] envp[7]="HOME=/var/spool/cups/tmp" D [06/Nov/2024:07:34:56 +0100] [Job 18] envp[8]="PATH=/usr/lib/cups/filter:/usr/bin:/usr/bin:/bin:/usr/bin" D [06/Nov/2024:07:34:56 +0100] [Job 18] envp[9]="SERVER_ADMIN=root@t14s" D [06/Nov/2024:07:34:56 +0100] [Job 18] envp[10]="SOFTWARE=CUPS/2.4.11" D [06/Nov/2024:07:34:56 +0100] [Job 18] envp[11]="TMPDIR=/var/spool/cups/tmp" D [06/Nov/2024:07:34:56 +0100] [Job 18] envp[12]="USER=root" D [06/Nov/2024:07:34:56 +0100] [Job 18] envp[13]="CUPS_MAX_MESSAGE=2047" D [06/Nov/2024:07:34:56 +0100] [Job 18] envp[14]="CUPS_SERVER=/run/cups/cups.sock" D [06/Nov/2024:07:34:56 +0100] [Job 18] envp[15]="CUPS_ENCRYPTION=IfRequested" D [06/Nov/2024:07:34:56 +0100] [Job 18] envp[16]="IPP_PORT=631" D [06/Nov/2024:07:34:56 +0100] [Job 18] envp[17]="CHARSET=utf-8" D [06/Nov/2024:07:34:56 +0100] [Job 18] envp[18]="LANG=en_US.UTF-8" D [06/Nov/2024:07:34:56 +0100] [Job 18] envp[19]="PPD=/etc/cups/ppd/ENVY_4520.ppd" D [06/Nov/2024:07:34:56 +0100] [Job 18] envp[20]="CONTENT_TYPE=text/plain" D [06/Nov/2024:07:34:56 +0100] [Job 18] envp[21]="DEVICE_URI=ipps://192.168.1.2:443/ipp/print" D [06/Nov/2024:07:34:56 +0100] [Job 18] envp[22]="PRINTER_INFO=HP ENVY 4520" D [06/Nov/2024:07:34:56 +0100] [Job 18] envp[23]="PRINTER_LOCATION=Home" D [06/Nov/2024:07:34:56 +0100] [Job 18] envp[24]="PRINTER=ENVY_4520" D [06/Nov/2024:07:34:56 +0100] [Job 18] envp[25]="PRINTER_STATE_REASONS=none" D [06/Nov/2024:07:34:56 +0100] [Job 18] envp[26]="CUPS_FILETYPE=document" D [06/Nov/2024:07:34:56 +0100] [Job 18] envp[27]="FINAL_CONTENT_TYPE=application/vnd.cups-raster" D [06/Nov/2024:07:34:56 +0100] [Job 18] envp[28]="AUTH_I****" D [06/Nov/2024:07:34:56 +0100] [Job 18] Started filter /usr/lib/cups/filter/universal (PID 20446) D [06/Nov/2024:07:34:56 +0100] [Job 18] Started filter /usr/lib/cups/filter/hpcups (PID 20447) D [06/Nov/2024:07:34:56 +0100] [Job 18] Started backend /usr/lib/cups/backend/ipps (PID 20448) D [06/Nov/2024:07:34:56 +0100] [Job 18] Sending stdin for job... D [06/Nov/2024:07:34:56 +0100] [Job 18] STATE: +connecting-to-device D [06/Nov/2024:07:34:56 +0100] [Job 18] Looking up \"192.168.1.2\"... D [06/Nov/2024:07:34:56 +0100] [Job 18] STATE: -connecting-to-device D [06/Nov/2024:07:34:56 +0100] [Job 18] 192.168.1.2=192.168.1.2 D [06/Nov/2024:07:34:56 +0100] [Job 18] ppdFilterLoadPPD: Color profile qualifier determined from job and PPD data \'RGB.Plain.\' D [06/Nov/2024:07:34:56 +0100] [Job 18] ppdFilterLoadPPD: No ICC profiles specified in PPD D [06/Nov/2024:07:34:56 +0100] [Job 18] ppdFilterLoadPPD: Searching for profile \"-/Plain\"... D [06/Nov/2024:07:34:56 +0100] [Job 18] prnt/hpcups/HPCupsFilter.cpp 592: cupsRasterOpen failed, fd = 0 D [06/Nov/2024:07:34:56 +0100] [Job 18] PID 20446 (/usr/lib/cups/filter/universal) crashed on signal 4. D [06/Nov/2024:07:34:56 +0100] [Job 18] Hint: Try setting the LogLevel to "debug" to find out more. D [06/Nov/2024:07:34:56 +0100] [Job 18] PID 20447 (/usr/lib/cups/filter/hpcups) stopped with status 1. D [06/Nov/2024:07:34:56 +0100] [Job 18] Hint: Try setting the LogLevel to "debug" to find out more. D [06/Nov/2024:07:34:56 +0100] [Job 18] hrDeviceDesc=\"ENVY 4520 series\" D [06/Nov/2024:07:34:56 +0100] [Job 18] prtMarkerColorantValue.1.1 = \"tri-color ink cartridge\" D [06/Nov/2024:07:34:56 +0100] [Job 18] prtMarkerColorantValue.1.2 = \"black ink cartridge\" D [06/Nov/2024:07:34:56 +0100] [Job 18] ATTR: marker-colors=none,none D [06/Nov/2024:07:34:56 +0100] [Job 18] ATTR: marker-names=\'\"tri-color ink cartridge HP unknown\"\',\'\"black ink cartridge HP unknown\"\' D [06/Nov/2024:07:34:56 +0100] [Job 18] ATTR: marker-types=ink,ink D [06/Nov/2024:07:34:56 +0100] [Job 18] ATTR: marker-levels=-1,-1 D [06/Nov/2024:07:34:56 +0100] [Job 18] new_supply_state=0, change_state=ffff D [06/Nov/2024:07:34:56 +0100] [Job 18] STATE: -developer-low-report D [06/Nov/2024:07:34:56 +0100] [Job 18] STATE: -developer-empty-warning D [06/Nov/2024:07:34:56 +0100] [Job 18] STATE: -marker-supply-low-report D [06/Nov/2024:07:34:56 +0100] [Job 18] STATE: -marker-supply-empty-warning D [06/Nov/2024:07:34:56 +0100] [Job 18] STATE: -opc-near-eol-report D [06/Nov/2024:07:34:56 +0100] [Job 18] STATE: -opc-life-over-warning D [06/Nov/2024:07:34:56 +0100] [Job 18] STATE: -toner-low-report D [06/Nov/2024:07:34:56 +0100] [Job 18] STATE: -toner-empty-warning D [06/Nov/2024:07:34:56 +0100] [Job 18] STATE: -waste-receptacle-almost-full-report D [06/Nov/2024:07:34:56 +0100] [Job 18] STATE: -waste-receptacle-full-warning D [06/Nov/2024:07:34:56 +0100] [Job 18] STATE: -cleaner-life-almost-over-report D [06/Nov/2024:07:34:56 +0100] [Job 18] STATE: -cleaner-life-over-warning D [06/Nov/2024:07:34:56 +0100] [Job 18] new_state=0, change_state=ffff D [06/Nov/2024:07:34:56 +0100] [Job 18] STATE: -media-empty-warning D [06/Nov/2024:07:34:56 +0100] [Job 18] STATE: -door-open-report D [06/Nov/2024:07:34:56 +0100] [Job 18] STATE: -media-jam-warning D [06/Nov/2024:07:34:56 +0100] [Job 18] STATE: -input-tray-missing-warning D [06/Nov/2024:07:34:56 +0100] [Job 18] STATE: -output-tray-missing-warning D [06/Nov/2024:07:34:56 +0100] [Job 18] STATE: -marker-supply-missing-warning D [06/Nov/2024:07:34:56 +0100] [Job 18] STATE: -output-area-almost-full-report D [06/Nov/2024:07:34:56 +0100] [Job 18] STATE: -output-area-full-warning D [06/Nov/2024:07:34:56 +0100] [Job 18] backendWaitLoop(snmp_fd=5, addr=0x7727dd1b1a88, side_cb=0x5e88371d3c90) D [06/Nov/2024:07:34:56 +0100] [Job 18] PID 20448 (/usr/lib/cups/backend/ipps) exited with no errors. D [06/Nov/2024:07:34:56 +0100] [Job 18] End of messages D [06/Nov/2024:07:34:56 +0100] [Job 18] printer-state=3(idle) D [06/Nov/2024:07:34:56 +0100] [Job 18] printer-state-message="Filter failed" D [06/Nov/2024:07:34:56 +0100] [Job 18] printer-state-reasons=none E [06/Nov/2024:07:42:35 +0100] [Job 19] Job stopped due to filter errors; please consult the /var/log/cups/error_log file for details. D [06/Nov/2024:07:42:35 +0100] [Job 19] The following messages were recorded from 07:42:35 to 07:42:35

tillkamppeter commented 3 weeks ago

@dcoppa Could you try with my suggested memmove()?

dcoppa commented 3 weeks ago

I've just tested, and I can confirm that printing works fine also with "memmove(buf, ptr, strnlen(ptr, sizeof(buf) - 1) + 1);". Thank you.

tillkamppeter commented 3 weeks ago

Starting at line 540 we have

     if (*ptr)
    *ptr = '\0';
      // Check whether the second word is not the cost value, then we have
      // a "*cupsFilter2:* line and the second word is the printer's input
      // format
      ptr ++;

Before doing the ptr ++; we do not check whether we are already beyond the terminating zero byte of the string in buf. If in kline 540 (the if (*ptr)) ptr had already pointed to the terminating zero (forged PPD with only one word as argument string of *cupsFilter(2): ... line), we do move ptr beyond the string with the ptr ++;

To solve this, this snipped should be changed into

      if (*ptr)
      {
    *ptr = '\0';
    // Check whether the second word is not the cost value, then we have
    // a "*cupsFilter2:* line and the second word is the printer's input
    // format
    ptr ++;
      }
dcoppa commented 3 weeks ago

I'm running with this now, and everything works:

diff -ur a/ppd/ppd-ipp.c b/ppd/ppd-ipp.c
--- a/ppd/ppd-ipp.c 2024-10-18 00:15:03.000000000 +0200
+++ b/ppd/ppd-ipp.c 2024-11-06 15:13:28.860584389 +0100
@@ -538,16 +538,18 @@
       ptr = buf;
       while (*ptr && !isspace(*ptr)) ptr ++;
       if (*ptr)
+      {
    *ptr = '\0';

       // Check whether the second word is not the cost value, then we have
       // a "*cupsFilter2:* line and the second word is the printer's input
       // format
       ptr ++;
+      }
       while (*ptr && isspace(*ptr)) ptr ++;
       if (!isdigit(*ptr))
       {
-   strcpy(buf, ptr);
+   memmove(buf, ptr, strnlen(ptr, sizeof(buf) - 1) + 1);
    ptr = buf;
    while (*ptr && !isspace(*ptr)) ptr ++;
    if (*ptr)
tillkamppeter commented 3 weeks ago

Applied these changes in commit 7da4e588

@dcoppa thanks a lot for the report.

I'm running with this now, and everything works:

diff -ur a/ppd/ppd-ipp.c b/ppd/ppd-ipp.c
--- a/ppd/ppd-ipp.c   2024-10-18 00:15:03.000000000 +0200
+++ b/ppd/ppd-ipp.c   2024-11-06 15:13:28.860584389 +0100
@@ -538,16 +538,18 @@
       ptr = buf;
       while (*ptr && !isspace(*ptr)) ptr ++;
       if (*ptr)
+      {
  *ptr = '\0';

       // Check whether the second word is not the cost value, then we have
       // a "*cupsFilter2:* line and the second word is the printer's input
       // format
       ptr ++;
+      }
       while (*ptr && isspace(*ptr)) ptr ++;
       if (!isdigit(*ptr))
       {
- strcpy(buf, ptr);
+ memmove(buf, ptr, strnlen(ptr, sizeof(buf) - 1) + 1);
  ptr = buf;
  while (*ptr && !isspace(*ptr)) ptr ++;
  if (*ptr)