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