From 661e8d8fb8248c38d67958beda32f3a5876d0c3f Mon Sep 17 00:00:00 2001

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

Date: Wed, 14 Nov 2018 09:50:08 +0000

Subject: [PATCH 1/4] Bug 700176: check the *output* device for

 LockSafetyParams

When calling .setdevice we were checking if LockSafetyParams was set, and if so

throwing an invalidaccess error.

The problem is, if another device, for example the pdf14 compositor is the 'top'

device, that does not (and cannot) honour LockSafetyParams.

To solve this, we'll now use the (relatively new) gxdso_current_output_device

spec_op to retrieve the *actual* output device, and check the LockSafetyParams

flag in that.

---

 psi/zdevice.c | 12 ++++++++----

 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/psi/zdevice.c b/psi/zdevice.c

index 8d48b74..c276746 100644

--- a/psi/zdevice.c

+++ b/psi/zdevice.c

@@ -461,13 +461,17 @@ zputdeviceparams(i_ctx_t *i_ctx_p)

 int

 zsetdevice(i_ctx_t *i_ctx_p)

 {

-    gx_device *dev = gs_currentdevice(igs);

+    gx_device *odev = NULL, *dev = gs_currentdevice(igs);

     os_ptr op = osp;

-    int code = 0;

+    int code = dev_proc(dev, dev_spec_op)(dev,

+                        gxdso_current_output_device, (void *)&odev, 0);

+

+    if (code < 0)

+        return code;

     check_write_type(*op, t_device);

-    if (dev->LockSafetyParams) {	  /* do additional checking if locked  */

-        if(op->value.pdevice != dev) 	  /* don't allow a different device    */

+    if (odev->LockSafetyParams) {	  /* do additional checking if locked  */

+        if(op->value.pdevice != odev) 	  /* don't allow a different device    */

             return_error(gs_error_invalidaccess);

     }

     dev->ShowpageCount = 0;

-- 

2.17.2

From ea1b3ef437f39e45874f821c06bd953196625ac5 Mon Sep 17 00:00:00 2001

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

Date: Wed, 14 Nov 2018 21:04:46 +0000

Subject: [PATCH 2/4] Bug 700176: Use the actual output device for both devices

 in setdevice

Also fixes bug 700189.

The pdf14 compositor device, despite being a forwarding device, does not forward

all spec_ops to it's target, only a select few are special cased for that.

gxdso_current_output_device needs to be included in those special cases.

The original commit (661e8d8fb8248) changed the code to use the spec_op to

retrieve the output device, checking that for LockSafetyParams. If

LockSafetyParams is set, it returns an invalidaccess error if the new device

differs from the current device.

When we do the comparison between the two devices, we need to check the

output device in both cases.

This is complicated by the fact that the new device may not have ever been set

(and thus fully initialised), and may not have a spec_op method available at

that point.

---

 base/gdevp14.c |  3 ++-

 psi/zdevice.c  | 18 ++++++++++++++++--

 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/base/gdevp14.c b/base/gdevp14.c

index f89bc04..f47ed30 100644

--- a/base/gdevp14.c

+++ b/base/gdevp14.c

@@ -5618,7 +5618,8 @@ pdf14_dev_spec_op(gx_device *pdev, int dev_spec_op,

             return 0;

         }

     }

-    if (dev_spec_op == gxdso_get_dev_param || dev_spec_op == gxdso_restrict_bbox) {

+    if (dev_spec_op == gxdso_get_dev_param || dev_spec_op == gxdso_restrict_bbox

+        || dev_spec_op == gxdso_current_output_device) {

         return dev_proc(p14dev->target, dev_spec_op)(p14dev->target, dev_spec_op, data, size);

     }

diff --git a/psi/zdevice.c b/psi/zdevice.c

index c276746..4beda04 100644

--- a/psi/zdevice.c

+++ b/psi/zdevice.c

@@ -462,16 +462,30 @@ int

 zsetdevice(i_ctx_t *i_ctx_p)

 {

     gx_device *odev = NULL, *dev = gs_currentdevice(igs);

+    gx_device *ndev = NULL;

     os_ptr op = osp;

     int code = dev_proc(dev, dev_spec_op)(dev,

                         gxdso_current_output_device, (void *)&odev, 0);

     if (code < 0)

         return code;

-

     check_write_type(*op, t_device);

+

+    /* slightly icky special case: the new device may not have had

+     * it's procs initialised, at this point - but we need to check

+     * whether we're being asked to change the device here

+     */

+    if (dev_proc((op->value.pdevice), dev_spec_op) == NULL)

+        ndev = op->value.pdevice;

+    else

+        code = dev_proc((op->value.pdevice), dev_spec_op)(op->value.pdevice,

+                        gxdso_current_output_device, (void *)&ndev, 0);

+

+    if (code < 0)

+        return code;

+

     if (odev->LockSafetyParams) {	  /* do additional checking if locked  */

-        if(op->value.pdevice != odev) 	  /* don't allow a different device    */

+        if(ndev != odev) 	  /* don't allow a different device    */

             return_error(gs_error_invalidaccess);

     }

     dev->ShowpageCount = 0;

-- 

2.17.2

From 7c3e7eee829cc3d2582e4aa7ae1fd495ca72cef1 Mon Sep 17 00:00:00 2001

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

Date: Mon, 17 Sep 2018 14:06:12 +0100

Subject: [PATCH 3/4] Implement .currentoutputdevice operator

The currentdevice operator returns the device currently installed in the

graphics state. This can be the output/page device, but also could be a

forwarding device (bbox device), compositor (pdf14) or subclass device

(erasepage optimisation, First/Last page etc).

In certain circumstances (for example during a setpagedevice) we want to be

sure we're retrieving the *actual* output/page device.

The new .currentoutputdevice operator uses the spec_op device method to traverse

any chain of devices and retrieve the final device in the chain, which

should always be the output/page device.

---

 Resource/Init/gs_init.ps  |  2 +-

 Resource/Init/gs_setpd.ps |  8 +++++++-

 base/gdevdflt.c           |  5 +++++

 base/gxdevsop.h           |  4 ++++

 psi/zdevice.c             | 30 ++++++++++++++++++++++++++++++

 5 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/Resource/Init/gs_init.ps b/Resource/Init/gs_init.ps

index bec307d..55d6923 100644

--- a/Resource/Init/gs_init.ps

+++ b/Resource/Init/gs_init.ps

@@ -2211,7 +2211,7 @@ SAFER { .setsafeglobal } if

   /.shfill /.argindex /.bytestring /.namestring /.stringbreak /.stringmatch /.globalvmarray /.globalvmdict /.globalvmpackedarray /.globalvmstring

   /.localvmarray /.localvmdict /.localvmpackedarray /.localvmstring /.systemvmarray /.systemvmdict /.systemvmpackedarray /.systemvmstring /.systemvmfile /.systemvmlibfile

   /.systemvmSFD /.settrapparams /.currentsystemparams /.currentuserparams /.getsystemparam /.getuserparam /.setsystemparams /.setuserparams

-  /.checkpassword /.locale_to_utf8 /.currentglobal /.gcheck /.imagepath

+  /.checkpassword /.locale_to_utf8 /.currentglobal /.gcheck /.imagepath /.currentoutputdevice

   % Used by a free user in the Library of Congress. Apparently this is used to

   % draw a partial page, which is then filled in by the results of a barcode

diff --git a/Resource/Init/gs_setpd.ps b/Resource/Init/gs_setpd.ps

index 8fa7c51..aa79b3f 100644

--- a/Resource/Init/gs_setpd.ps

+++ b/Resource/Init/gs_setpd.ps

@@ -877,7 +877,13 @@ SETPDDEBUG { (Selecting.) = pstack flush } if

                 % Stack: mark <orig> <request> <merged> <failed>

 SETPDDEBUG { (Constructing.) = pstack flush } if

-   currentdevice .devicename 2 index /OutputDevice get eq

+   % Non-obvious: we need to check the name of the output device, to tell

+   % whether we're going to have to replace the entire device chain (which

+   % may be only one device, or may be multiple devices.

+   % If we're not replacing the entire change, we have to use the device in

+   % the graphics state, so the configuration of the entire device chain is

+   % correctly set.

+   .currentoutputdevice .devicename 2 index /OutputDevice get eq

     { currentdevice }

     { 1 index /OutputDevice get finddevice }

    ifelse

diff --git a/base/gdevdflt.c b/base/gdevdflt.c

index 3cb9fbd..b5bd82b 100644

--- a/base/gdevdflt.c

+++ b/base/gdevdflt.c

@@ -1044,6 +1044,11 @@ gx_default_dev_spec_op(gx_device *pdev, int dev_spec_op, void *data, int size)

                 dev_param_req_t *request = (dev_param_req_t *)data;

                 return gx_default_get_param(pdev, request->Param, request->list);

             }

+        case gxdso_current_output_device:

+            {

+                *(gx_device **)data = pdev;

+                return 0;

+            }

     }

     return_error(gs_error_undefined);

 }

diff --git a/base/gxdevsop.h b/base/gxdevsop.h

index cd3b632..27e3e84 100644

--- a/base/gxdevsop.h

+++ b/base/gxdevsop.h

@@ -327,6 +327,10 @@ enum {

     gxdso_JPEG_passthrough_data,

     gxdso_JPEG_passthrough_end,

     gxdso_supports_iccpostrender,

+    /* Retrieve the last device in a device chain

+       (either forwarding or subclass devices).

+     */

+    gxdso_current_output_device,

     /* Add new gxdso_ keys above this. */

     gxdso_pattern__LAST

 };

diff --git a/psi/zdevice.c b/psi/zdevice.c

index 4beda04..03285dc 100644

--- a/psi/zdevice.c

+++ b/psi/zdevice.c

@@ -57,6 +57,7 @@ zcopydevice2(i_ctx_t *i_ctx_p)

 }

 /* - currentdevice <device> */

+/* Returns the current device in the graphics state */

 int

 zcurrentdevice(i_ctx_t *i_ctx_p)

 {

@@ -71,6 +72,34 @@ zcurrentdevice(i_ctx_t *i_ctx_p)

     return 0;

 }

+/* - .currentoutputdevice <device> */

+/* Returns the *output* device - which will often

+   be the same as above, but not always: if a compositor

+   or other forwarding device, or subclassing device is

+   in force, that will be referenced by the graphics state

+   rather than the output device.

+   This is equivalent of currentdevice device, but returns

+   the *device* object, rather than the dictionary describing

+   the device and device state.

+ */

+static int

+zcurrentoutputdevice(i_ctx_t *i_ctx_p)

+{

+    os_ptr op = osp;

+    gx_device *odev = NULL, *dev = gs_currentdevice(igs);

+    gs_ref_memory_t *mem = (gs_ref_memory_t *) dev->memory;

+    int code = dev_proc(dev, dev_spec_op)(dev,

+                        gxdso_current_output_device, (void *)&odev, 0);

+    if (code < 0)

+        return code;

+

+    push(1);

+    make_tav(op, t_device,

+             (mem == 0 ? avm_foreign : imemory_space(mem)) | a_all,

+             pdevice, odev);

+    return 0;

+}

+

 /* <device> .devicename <string> */

 static int

 zdevicename(i_ctx_t *i_ctx_p)

@@ -632,6 +661,7 @@ const op_def zdevice_op_defs[] =

 {

     {"1.copydevice2", zcopydevice2},

     {"0currentdevice", zcurrentdevice},

+    {"0.currentoutputdevice", zcurrentoutputdevice},

     {"1.devicename", zdevicename},

     {"0.doneshowpage", zdoneshowpage},

     {"0flushpage", zflushpage},

-- 

2.17.2

From 4d98293c72cc5b5fe456065a3252d39e9ab28e4d Mon Sep 17 00:00:00 2001

From: Ken Sharp <ken.sharp@artifex.com>

Date: Mon, 19 Nov 2018 09:00:54 +0000

Subject: [PATCH 4/4] Coverity ID 327264 - move pointer NULL check

Due to recent changes in this code, the pointer was being dereferenced

before we checked it to see if it was NULL. Moe the check so that we

check for NULL before dereferencing.

The 'pvalue' of the operand can be NULL, even if the object is a t_device

type, because invalidate_stack_devices traverses the operand stack

looking for devices, and sets their pvalue member to NULL in order to

invalidate them so that they cannot be used.

---

 psi/zdevice.c | 6 +++---

 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/psi/zdevice.c b/psi/zdevice.c

index 03285dc..63865f1 100644

--- a/psi/zdevice.c

+++ b/psi/zdevice.c

@@ -500,6 +500,9 @@ zsetdevice(i_ctx_t *i_ctx_p)

         return code;

     check_write_type(*op, t_device);

+    if (op->value.pdevice == 0)

+        return gs_note_error(gs_error_undefined);

+

     /* slightly icky special case: the new device may not have had

      * it's procs initialised, at this point - but we need to check

      * whether we're being asked to change the device here

@@ -519,9 +522,6 @@ zsetdevice(i_ctx_t *i_ctx_p)

     }

     dev->ShowpageCount = 0;

-    if (op->value.pdevice == 0)

-        return gs_note_error(gs_error_undefined);

-

     code = gs_setdevice_no_erase(igs, op->value.pdevice);

     if (code < 0)

         return code;

-- 

2.17.2