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[MAXPARAMS]; / 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 ++;