Control: found -1 3.10.6-2+squeeze1
Control: found -1 3.12.6-3
Control: found -1 3.12.11-1

All versions that are currently in the archive are affected by this
bug.

On 2013-02-22 15:15:13, Moritz Muehlenhoff wrote:
> Package: hplip
> Severity: grave
> Tags: security
> Justification: user security hole
> 
> Several further insecurely handled temporary files were discovered by Red Hat:
> https://www.redhat.com/archives/enterprise-watch-list/2013-February/msg00024.html
> 
> I've extracted the patch from the RHEL update, it's attached to this mail.

The patch introduces one buffer overflow and an regression.

> diff -up hplip-3.12.4/prnt/hpcups/HPCupsFilter.cpp.CVE-2013-0200 
> hplip-3.12.4/prnt/hpcups/HPCupsFilter.cpp
> --- hplip-3.12.4/prnt/hpcups/HPCupsFilter.cpp.CVE-2013-0200   2013-01-22 
> 10:57:13.651460928 +0000
> +++ hplip-3.12.4/prnt/hpcups/HPCupsFilter.cpp 2013-01-22 10:57:34.087541538 
> +0000
> @@ -637,19 +637,22 @@ int HPCupsFilter::processRasterData(cups
>          {
>              char    szFileName[32];
>              memset(szFileName, 0, sizeof(szFileName));
> -            snprintf (szFileName, sizeof(szFileName), 
> "/tmp/hpcupsfilterc_%d.bmp", current_page_number);
> +            snprintf (szFileName, sizeof(szFileName), 
> "/tmp/hpcupsfilterc_%d.bmp.XXXXXX", current_page_number);

If current_page_number is larger than 9, the last six characters of
szFileName won't be XXXXXX and hence mkstemp will fail with EINVAL.

> diff -up hplip-3.12.4/prnt/hpcups/SystemServices.cpp.CVE-2013-0200 
> hplip-3.12.4/prnt/hpcups/SystemServices.cpp
> --- hplip-3.12.4/prnt/hpcups/SystemServices.cpp.CVE-2013-0200 2012-04-10 
> 09:32:37.000000000 +0100
> +++ hplip-3.12.4/prnt/hpcups/SystemServices.cpp       2013-01-22 
> 10:57:34.088541545 +0000
> @@ -36,10 +36,12 @@ SystemServices::SystemServices(int iLogL
>      m_fp = NULL;
>      if (iLogLevel & SAVE_PCL_FILE)
>      {
> +     int     fd;
>          char    fname[32];
> -        sprintf(fname, "/tmp/hpcups_job%d.out", job_id);
> -        m_fp = fopen(fname, "w");
> -        chmod(fname, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
> +        sprintf(fname, "/tmp/hpcups_job%d.out.XXXXXX", job_id);

job_id > 100000 will cause a buffer overflow. According to cups' API
documentation job_id can be up to 2^31-1 [1].

The attached patch makes the buffers large enough. I'll prepare a NMU
later today if nobody beats me to it.

Regards

[1] http://www.cups.org/documentation.php/api-cups.html#PRINT_JOBS
-- 
Sebastian Ramacher
--- a/prnt/hpcups/HPCupsFilter.cpp
+++ b/prnt/hpcups/HPCupsFilter.cpp
@@ -656,21 +656,24 @@
         
         if (m_iLogLevel & SAVE_INPUT_RASTERS)
         {
-            char    szFileName[32];
+            char    szFileName[44];
             memset(szFileName, 0, sizeof(szFileName));
-            snprintf (szFileName, sizeof(szFileName), "/tmp/hpcupsfilterc_%d.bmp", current_page_number);
+            snprintf (szFileName, sizeof(szFileName), "/tmp/hpcupsfilterc_%d.bmp.XXXXXX", current_page_number);
             if (cups_header.cupsColorSpace == CUPS_CSPACE_RGBW ||
                 cups_header.cupsColorSpace == CUPS_CSPACE_RGB)
             {
-                cfp = fopen (szFileName, "w");
-                chmod (szFileName, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+		int fd = mkstemp (szFileName);
+		if (fd != -1)
+		    cfp = fdopen (fd, "w");
             }
             if (cups_header.cupsColorSpace == CUPS_CSPACE_RGBW ||
                 cups_header.cupsColorSpace == CUPS_CSPACE_K)
             {
-                szFileName[17] = 'k';
-                kfp = fopen (szFileName, "w");
-                chmod (szFileName, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+		int fd;
+		snprintf (szFileName, sizeof(szFileName), "/tmp/hpcupsfilterk_%d.bmp.XXXXXX", current_page_number);
+		fd = mkstemp (szFileName);
+		if (fd != -1)
+		    kfp = fdopen (fd, "w");
             }
 
             WriteBMPHeader (cfp, cups_header.cupsWidth, cups_header.cupsHeight, COLOR_RASTER);
--- a/prnt/hpcups/SystemServices.cpp
+++ b/prnt/hpcups/SystemServices.cpp
@@ -36,10 +36,12 @@
     m_fp = NULL;
     if (iLogLevel & SAVE_PCL_FILE)
     {
-        char    fname[32];
-        sprintf(fname, "/tmp/hpcups_job%d.out", job_id);
-        m_fp = fopen(fname, "w");
-        chmod(fname, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+	int	fd;
+        char    fname[40];
+        sprintf(fname, "/tmp/hpcups_job%d.out.XXXXXX", job_id);
+	fd = mkstemp (fname);
+	if (fd != -1)
+	    m_fp = fdopen(fd, "w");
     }
 }
 
--- a/prnt/hpijs/hpijs.cpp
+++ b/prnt/hpijs/hpijs.cpp
@@ -96,13 +96,12 @@
 
     if (pSS->m_iLogLevel & SAVE_PCL_FILE)
     {
+	int	fd;
         char    szFileName[32];
-	sprintf (szFileName, "/tmp/hpijs_%d.out", getpid());
-	pSS->outfp = fopen (szFileName, "w");
-	if (pSS->outfp)
-	{
-	    chmod (szFileName, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
-	}
+	sprintf (szFileName, "/tmp/hpijs_%d.out.XXXXXX", getpid());
+	fd = mkstemp (szFileName);
+	if (fd != -1)
+	    pSS->outfp = fdopen (fd, "w");
     }
 }
 
--- a/prnt/hpps/hppsfilter.c
+++ b/prnt/hpps/hppsfilter.c
@@ -92,10 +92,12 @@
     g_fp_outdbgps = NULL;
     if (g_savepsfile & SAVE_PS_FILE)
     {
+	int	fd;
         char    sfile_name[FILE_NAME_SIZE] = {0};
-        sprintf(sfile_name, DBG_PSFILE, szjob_id);
-        g_fp_outdbgps= fopen(sfile_name, "w");
-        chmod(sfile_name, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+        sprintf(sfile_name, DBG_PSFILE ".XXXXXX", szjob_id);
+	fd = mkstemp (sfile_name);
+	if (fd != -1)
+	    g_fp_outdbgps = fdopen(fd, "w");
     }
 }
 

Attachment: signature.asc
Description: Digital signature

Reply via email to