From 5875d32ce071e591461e404bdd8aae849ccdcab1 Mon Sep 17 00:00:00 2001
From: Matthias Gerstner <matthias.gerstner@suse.de>
Date: Fri, 8 Sep 2023 10:17:04 +0200
Subject: [PATCH] hppsfilter: booklet printing: change insecure fixed /tmp file
 paths

Using the fixed /tmp file paths in booklet printing /tmp/booklet.ps,
/tmp/temp.ps and /tmp/NUP.ps is a local security issue and also prevents
potential parallel operation of hplip.

Use proper `mkstemp()` for these files. Functions like `PS_Booklet()`
and `cupsFileOpen()` don't use the open file descriptor but open the
path by name again. This is safe, since the files have already been
safely created and have safe modes. I wanted to avoid changing a whole
series of function signatures for this.

The purpose of the `chmod()` in `open_tempbookletfile()` is unclear, the
data should only be processed by our own process. Making the file world
readable is an information leak, though. Thus drop this line.
---
 prnt/hpps/hppsfilter.c | 124 ++++++++++++++++++++++++++++++++---------
 1 file changed, 98 insertions(+), 26 deletions(-)

diff --git a/prnt/hpps/hppsfilter.c b/prnt/hpps/hppsfilter.c
index d6721b1..711b8d8 100644
--- a/prnt/hpps/hppsfilter.c
+++ b/prnt/hpps/hppsfilter.c
@@ -43,7 +43,9 @@ static FILE *g_fp_outdbgps = NULL;
 static FILE *ptempbooklet_file = NULL;
 static char temp_filename[FILE_NAME_SIZE] = {0};
 static char booklet_filename[FILE_NAME_SIZE] = {0};
+static int booklet_fd = -1;
 static char Nup_filename[FILE_NAME_SIZE] = {0};
+static int Nup_fd = -1;
 extern void PS_Booklet(char *tempfile, char *bookletfile, char *nupfile,int order, int nup, char* pagesize, int bookletMaker);
 static const char *GetOptionValue(const char *iOptionValue);
 
@@ -99,16 +101,78 @@ static int hpwrite (void *pBuffer, size_t size)
     return ndata_written;
 }
 
-static void open_tempbookletfile(char *mode)
+static int open_tempbookletfile(char *mode)
 {
-    ptempbooklet_file= fopen(temp_filename, mode);
+    snprintf(temp_filename, FILE_NAME_SIZE, "/tmp/hppsfilter-temp.XXXXXX");
+    int fd = mkstemp(temp_filename);
+    if (fd < 0) {
+        temp_filename[0] = '\0';
+        fprintf(stderr, "ERROR: Unable to open temp file %s\n", temp_filename);
+        return 1;
+    }
+
+    ptempbooklet_file = fdopen(fd, mode);
     if(ptempbooklet_file == NULL)
     {
-            fprintf(stderr, "ERROR: Unable to open temp file %s\n", temp_filename);
-            return 1;
+        close(fd);
+        fprintf(stderr, "ERROR: Unable to open temp file %s\n", temp_filename);
+        return 1;
     }  
-    chmod(temp_filename, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+    return 0;
+}
+
+static void clean_tempfiles()
+{
+    if (booklet_fd != -1)
+    {
+        close(booklet_fd);
+        booklet_fd = -1;
+    }
+
+    if (Nup_fd != -1)
+    {
+        close(Nup_fd);
+        Nup_fd = -1;
+    }
+
+    if (ptempbooklet_file != NULL)
+    {
+        fclose(ptempbooklet_file);
+        ptempbooklet_file = NULL;
+    }
+
+    if( booklet_filename[0] != '\0' )
+    {
+        if ((unlink(booklet_filename)) == -1)
+        {
+            fprintf(stderr, "ERROR: Unable to remove temporary files in /tmp dir \"%s\" ",booklet_filename);
+            return 1;
+        }
+
+        booklet_filename[0] = '\0';
+    }
+
+    if( temp_filename[0] != '\0' )
+    {
+        if ((unlink(temp_filename)) == -1)
+        {
+            fprintf(stderr, "ERROR: Unable to remove temporary files in /tmp dir \"%s\"  ",temp_filename);
+            return 1;
+        }
+
+        temp_filename[0] = '\0';
+    }
 
+    if( Nup_filename[0] != '\0' )
+    {
+        if ((unlink(Nup_filename)) == -1)
+        {
+            fprintf(stderr, "ERROR: Unable to remove temporary files in /tmp dir \"%s\" ",Nup_filename);
+            return 1;
+        }
+
+        Nup_filename[0] = '\0';
+    }
 }
 
 static int Dump_tempbookletfile (void *pBuffer, size_t size)
@@ -921,6 +985,8 @@ int main (int argc, char **argv)
    char buffer[MAX_BUFFER]     = {0};
    int LfpSecurePin = 0;
 
+   atexit(clean_tempfiles);
+
     get_LogLevel();
     setbuf (stderr, NULL);
 
@@ -1024,13 +1090,32 @@ int main (int argc, char **argv)
     if(booklet_enabled)
     {
         /* 1. dump  the contents of the input file into temp file */
-        sprintf(booklet_filename, "/tmp/%s.ps","booklet");
-        sprintf(temp_filename, "/tmp/%s.ps","temp");
-        sprintf(Nup_filename, "/tmp/%s.ps","NUP");
-        open_tempbookletfile("w");
-	while( (numBytes = cupsFileGetLine(fp_input, line, sizeof(line))) > 0)
+        snprintf(booklet_filename, FILE_NAME_SIZE, "/tmp/hppsfilter-booklet.XXXXXX");
+        booklet_fd = mkstemp(booklet_filename);
+        if( booklet_fd < 0 )
+        {
+            booklet_filename[0] = '\0';
+            fprintf(stderr, "ERROR: Unable to create booklet temporary file \"%s\"", booklet_filename);
+            return 1;
+        }
+
+        snprintf(Nup_filename, FILE_NAME_SIZE, "/tmp/hppsfilter-nup.XXXXXX");
+        Nup_fd = mkstemp(Nup_filename);
+        if( Nup_fd < 0 )
+        {
+            Nup_filename[0] = '\0';
+            clean_tempfiles();
+            fprintf(stderr, "ERROR: Unable to create nup temporary file \"%s\"", Nup_filename);
+            return 1;
+        }
+
+        if( open_tempbookletfile("w") != 0 )
+        {
+            clean_tempfiles();
+            return 1;
+        }
+        while( (numBytes = cupsFileGetLine(fp_input, line, sizeof(line))) > 0)
             Dump_tempbookletfile (line, numBytes);
-        fclose(ptempbooklet_file);
 
         /* 2. Perform the booklet operation on the PS file */
         PS_Booklet(temp_filename,booklet_filename,Nup_filename,order,nup,subString,bookletMaker);
@@ -1040,6 +1125,7 @@ int main (int argc, char **argv)
         if ((fp_bookletinput = cupsFileOpen(Nup_filename, "r")) == NULL)
         {
             fprintf(stderr, "ERROR: Unable to open Nup_filename print file \"%s\"", Nup_filename);
+            clean_tempfiles();
             return 1;
         }
         while ( (numBytes = cupsFileGetLine(fp_bookletinput, line, sizeof(line))) > 0)
@@ -1047,21 +1133,7 @@ int main (int argc, char **argv)
         cupsFileClose (fp_bookletinput);
 
         /* 4. Unlink function to remove the temp temporary files created */
-        if( (unlink(booklet_filename)) == -1)
-        {
-            fprintf(stderr, "ERROR: Unable to remove temporary files in /tmp dir \"%s\" ",booklet_filename);
-            return 1;
-        }
-        if( (unlink(temp_filename)) == -1)
-        {
-            fprintf(stderr, "ERROR: Unable to remove temporary files in /tmp dir \"%s\"  ",temp_filename);
-            return 1;
-        }
-        if( (unlink(Nup_filename)) == -1)
-        {
-            fprintf(stderr, "ERROR: Unable to remove temporary files in /tmp dir \"%s\" ",Nup_filename);
-            return 1;
-        }
+        clean_tempfiles();
         booklet_enabled = 0;
         bookletMaker=0;
     }
-- 
2.41.0

