CUPS 1.1.22 hpgltops ParseCommand

[michaelrsweet]

Version: 1.1.22
CUPS.org User: d.j.bernstein

Ariel Berkman, a student in my Fall 2004 UNIX Security Holes course, has
discovered a remotely exploitable security hole in CUPS. I'm publishing
this notice, but all the discovery credits should be assigned to
Berkman.

A CUPS installation is at risk whenever it prints an HPGL file obtained
from email (or a web page or any other source that could be controlled
by an attacker). You are at risk if you print data through a CUPS
installation at risk. The source of the HPGL file has complete control
over the CUPS ``lp'' account; in particular, he can read and modify the
files you are printing.

Proof of concept: On an x86 computer running FreeBSD 4.10, as root, type

cd /usr/ports/print/cups
make install

to download and compile the CUPS package, version 1.1.22 (current).
Then, as any user, save the file 21.hpgl.gz attached to this message,
and type

gunzip 21.hpgl
/usr/local/libexec/cups/filter/hpgltops
15 $USER test-title 1 none 21.hpgl > 21.ps

with the unauthorized result that a file named x is removed from the
current directory. (I tested this with a 541-byte environment, as
reported by printenv | wc -c.)

Here's the bug: In hpgl-input.c, ParseCommand() reads any number of
bytes into a 262144-byte buf[] array.

---D. J. Bernstein, Associate Professor, Department of Mathematics,
Statistics, and Computer Science, University of Illinois at Chicago

[michaelrsweet]

CUPS.org User: twaugh.redhat

How about the attached patch?

[michaelrsweet]

CUPS.org User: mike

Your patch missed the PE code below the LB code.

Also, we want to read up to the terminator, even if we can't store the whole thing...

str1024esp.patch will be part of 1.1.23rc1.

[michaelrsweet]

"cups-str1024.patch":

--- cups-1.1.22/filter/hpgl-input.c.str1024 2004-12-16 16:05:53.264940147 +0000
+++ cups-1.1.22/filter/hpgl-input.c 2004-12-16 16:07:23.251509102 +0000
@@ -128,7 +128,8 @@

if (strcasecmp(name, "LB") == 0)
{

• for (i = 0; (ch = getc(fp)) != StringTerminator; i ++)
• for (i = 0; i < (sizeof(buf) - 1) && (ch = getc(fp)) != StringTerminator;
• i ++)
  buf[i] = ch;
  buf[i] = '\0';
  p[num_params].type = PARAM_STRING;

[michaelrsweet]

"str1024esp.patch":

Index: hpgl-input.c

RCS file: /development/cvs/cups/filter/hpgl-input.c,v
retrieving revision 1.16
diff -u -r1.16 hpgl-input.c
--- hpgl-input.c 25 Feb 2004 20:14:52 -0000 1.16
+++ hpgl-input.c 16 Dec 2004 19:38:12 -0000
@@ -54,7 +54,8 @@
ch, /* Current char /
done, / Non-zero when the current command is read /
i; / Looping var */

• char buf[262144]; /* String buffer */
• char buf[262144], /* String buffer */

•   _bufptr;    /_ Pointer into buffer _/
  

  static param_t p[MAX_PARAMS]; /_ Parameter buffer */

@@ -128,9 +129,12 @@

if (strcasecmp(name, "LB") == 0)
{

• for (i = 0; (ch = getc(fp)) != StringTerminator; i ++)

•  buf[i] = ch;
  

• buf[i] = '\0';
• bufptr = buf;
• while ((ch = getc(fp)) != StringTerminator)

•  if (bufptr < (buf + sizeof(buf) - 1))
  

•    *bufptr++ = ch;
  

• *bufptr = '\0';

p[num_params].type = PARAM_STRING;
p[num_params].value.string = strdup(buf);
num_params ++;
@@ -155,11 +159,12 @@
}
else if (strcasecmp(name, "PE") == 0)
{

• for (i = 0; i < (sizeof(buf) - 1); i ++)

•  if ((buf[i] = getc(fp)) == ';')
  

•    break;
  

• bufptr = buf;
• while ((ch = getc(fp)) != ';')

•  if (bufptr < (buf + sizeof(buf) - 1))
  

•    *bufptr++ = ch;
  

• *bufptr = '\0';
• buf[i] = '\0';
  p[num_params].type = PARAM_STRING;
  p[num_params].value.string = strdup(buf);
  num_params ++;