From: Chris Liddell <chris.liddell@artifex.com>

Date: Tue, 21 Aug 2018 19:17:05 +0000 (+0100)

Subject: Bug 699657: properly apply file permissions to .tempfile

Bug 699657: properly apply file permissions to .tempfile

https://git.ghostscript.com/?p=ghostpdl.git;a=commit;h=0d3901189f245232f0161addf215d7268c4d05a3

From: Chris Liddell <chris.liddell@artifex.com>

Date: Tue, 21 Aug 2018 19:17:51 +0000 (+0100)

Subject: Bug 699658: Fix handling of pre-SAFER opened files.

Bug 699658: Fix handling of pre-SAFER opened files.

Temp files opened for writing before SAFER is engaged are not subject to the

SAFER restrictions - that is handled by recording in a dictionary, and

checking that as part of the permissions checks.

By adding a custom error handler for invalidaccess, that allowed the filename

to be added to the dictionary (despite the attempted open throwing the error)

thus meaning subsequent accesses were erroneously permitted.

https://git.ghostscript.com/?p=ghostpdl.git;a=commit;h=a054156d425b4dbdaaa9fda4b5f1182b27598c2b

---

diff -up a/psi/zfile.c.cve-2018-15908 b/psi/zfile.c

--- a/psi/zfile.c.cve-2018-15908	2018-11-14 15:13:31.249625819 +0100

+++ b/psi/zfile.c	2018-11-14 15:14:16.933831779 +0100

@@ -121,7 +121,7 @@ make_invalid_file(i_ctx_t *i_ctx_p, ref

 /* strings of the permitgroup array. */

 static int

 check_file_permissions_reduced(i_ctx_t *i_ctx_p, const char *fname, int len,

-                        const char *permitgroup)

+                        gx_io_device *iodev, const char *permitgroup)

 {

     long i;

     ref *permitlist = NULL;

@@ -131,8 +131,14 @@ check_file_permissions_reduced(i_ctx_t *

     bool use_windows_pathsep = (gs_file_name_check_separator(win_sep2, 1, win_sep2) == 1);

     uint plen = gp_file_name_parents(fname, len);

-    /* Assuming a reduced file name. */

+    /* we're protecting arbitrary file system accesses, not Postscript device accesses.

+     * Although, note that %pipe% is explicitly checked for and disallowed elsewhere

+     */

+    if (iodev && iodev != iodev_default(imemory)) {

+        return 0;

+    }

+    /* Assuming a reduced file name. */

     if (dict_find_string(&(i_ctx_p->userparams), permitgroup, &permitlist) <= 0)

         return 0;       /* if Permissions not found, just allow access */

@@ -187,14 +193,14 @@ check_file_permissions_reduced(i_ctx_t *

 /* strings of the permitgroup array */

 static int

 check_file_permissions(i_ctx_t *i_ctx_p, const char *fname, int len,

-                        const char *permitgroup)

+                        gx_io_device *iodev, const char *permitgroup)

 {

     char fname_reduced[gp_file_name_sizeof];

     uint rlen = sizeof(fname_reduced);

     if (gp_file_name_reduce(fname, len, fname_reduced, &rlen) != gp_combine_success)

         return e_invalidaccess;         /* fail if we couldn't reduce */

-    return check_file_permissions_reduced(i_ctx_p, fname_reduced, rlen, permitgroup);

+    return check_file_permissions_reduced(i_ctx_p, fname_reduced, rlen, iodev, permitgroup);

 }

 /* <name_string> <access_string> file <file> */

@@ -298,7 +304,7 @@ zdeletefile(i_ctx_t *i_ctx_p)

         return code;

     if (pname.iodev == iodev_default(imemory)) {

         if ((code = check_file_permissions(i_ctx_p, pname.fname, pname.len,

-                "PermitFileControl")) < 0 &&

+                pname.iodev, "PermitFileControl")) < 0 &&

                  !file_is_tempfile(i_ctx_p, op->value.bytes, r_size(op))) {

             return code;

         }

@@ -382,7 +388,7 @@ file_continue(i_ctx_t *i_ctx_p)

         } else if (code > len)      /* overran string */

             return_error(gs_error_rangecheck);

         else if (iodev != iodev_default(imemory)

-              || (check_file_permissions_reduced(i_ctx_p, (char *)pscratch->value.bytes, code + devlen, "PermitFileReading")) == 0) {

+              || (check_file_permissions_reduced(i_ctx_p, (char *)pscratch->value.bytes, code + devlen, NULL, "PermitFileReading")) == 0) {

             push(1);

             ref_assign(op, pscratch);

             r_set_size(op, code + devlen);

@@ -432,12 +438,12 @@ zrenamefile(i_ctx_t *i_ctx_p)

                  * and FileWriting permissions to the destination file/path.

                  */

               ((check_file_permissions(i_ctx_p, pname1.fname, pname1.len,

-                                        "PermitFileControl") < 0 &&

+                                        pname1.iodev, "PermitFileControl") < 0 &&

                   !file_is_tempfile(i_ctx_p, op[-1].value.bytes, r_size(op - 1))) ||

               (check_file_permissions(i_ctx_p, pname2.fname, pname2.len,

-                                        "PermitFileControl") < 0 ||

+                                        pname2.iodev, "PermitFileControl") < 0 ||

               check_file_permissions(i_ctx_p, pname2.fname, pname2.len,

-                                        "PermitFileWriting") < 0 )))) {

+                                        pname2.iodev, "PermitFileWriting") < 0 )))) {

             code = gs_note_error(e_invalidfileaccess);

         } else {

             code = (*pname1.iodev->procs.rename_file)(pname1.iodev,

@@ -484,8 +490,11 @@ zstatus(i_ctx_t *i_ctx_p)

                 code = gs_terminate_file_name(&pname, imemory, "status");

                 if (code < 0)

                     return code;

-                code = (*pname.iodev->procs.file_status)(pname.iodev,

+                if ((code = check_file_permissions(i_ctx_p, pname.fname, pname.len,

+                                       pname.iodev, "PermitFileReading")) >= 0) {

+                    code = (*pname.iodev->procs.file_status)(pname.iodev,

                                                        pname.fname, &fstat);

+                }

                 switch (code) {

                     case 0:

                         check_ostack(4);

@@ -694,8 +703,24 @@ ztempfile(i_ctx_t *i_ctx_p)

     }

     if (gp_file_name_is_absolute(pstr, strlen(pstr))) {

-        if (check_file_permissions(i_ctx_p, pstr, strlen(pstr),

-                                   "PermitFileWriting") < 0) {

+        int plen = strlen(pstr);

+        const char *sep = gp_file_name_separator();

+#ifdef DEBUG

+        int seplen = strlen(sep);

+        if (seplen != 1)

+            return_error(gs_error_Fatal);

+#endif

+        /* strip off the file name prefix, leave just the directory name

+         * so we can check if we are allowed to write to it

+         */

+        for ( ; plen >=0; plen--) {

+            if (pstr[plen] == sep[0])

+                break;

+        }

+        memcpy(fname, pstr, plen);

+        fname[plen] = '\0';

+        if (check_file_permissions(i_ctx_p, fname, strlen(fname),

+                                   NULL, "PermitFileWriting") < 0) {

             return_error(e_invalidfileaccess);

         }

     } else if (!prefix_is_simple(pstr)) {

@@ -837,6 +862,7 @@ zopen_file(i_ctx_t *i_ctx_p, const gs_pa

            const char *file_access, stream **ps, gs_memory_t *mem)

 {

     gx_io_device *const iodev = pfn->iodev;

+    int code = 0;

     if (pfn->fname == NULL)     /* just a device */

         return iodev->procs.open_device(iodev, file_access, ps, mem);

@@ -847,7 +873,7 @@ zopen_file(i_ctx_t *i_ctx_p, const gs_pa

             open_file = iodev_os_open_file;

         /* Check OS files to make sure we allow the type of access */

         if (open_file == iodev_os_open_file) {

-            int code = check_file_permissions(i_ctx_p, pfn->fname, pfn->len,

+            code = check_file_permissions(i_ctx_p, pfn->fname, pfn->len, pfn->iodev,

                 file_access[0] == 'r' ? "PermitFileReading" : "PermitFileWriting");

             if (code < 0 && !file_is_tempfile(i_ctx_p,

@@ -894,7 +920,7 @@ check_file_permissions_aux(i_ctx_t *i_ct

     /* fname must be reduced. */

     if (i_ctx_p == NULL)

         return 0;

-    if (check_file_permissions_reduced(i_ctx_p, fname, flen, "PermitFileReading") < 0)

+    if (check_file_permissions_reduced(i_ctx_p, fname, flen, NULL, "PermitFileReading") < 0)

         return_error(e_invalidfileaccess);

     return 0;

 }

diff -up a/Resource/Init/gs_init.ps.cve-2018-15908 b/Resource/Init/gs_init.ps

--- a/Resource/Init/gs_init.ps.cve-2018-15908	2018-11-14 16:34:23.268867657 +0100

+++ b/Resource/Init/gs_init.ps	2018-11-14 16:36:38.765552576 +0100

@@ -2015,6 +2015,19 @@ readonly def

             concatstrings concatstrings .generate_dir_list_templates

         } if

       ]

+      /PermitFileWriting [

+          currentuserparams /PermitFileWriting get aload pop

+          (TMPDIR) getenv not

+          {

+            (TEMP) getenv not

+            {

+              (TMP) getenv not

+              {

+                (/temp) (/tmp)

+              } if

+            } if

+          } if

+      ]

       /LockFilePermissions //true

     >> setuserparams

   }

@@ -2062,7 +2075,9 @@ readonly def

 % the file can be deleted later, even if SAFER is set.

 /.tempfile {

   .tempfile	% filename file

-  //SAFETY /tempfiles get 2 .argindex //true .forceput

+    //SAFETY /safe get not { % only add the filename if we're not yet safe

+    //SAFETY /tempfiles get 2 .argindex //true .forceput

+  } if

 } .bind executeonly odef

 % If we are running in SAFER mode, lock things down