Bug #699707 "Security review bug - continuation procedures"

As a result of the recent security review, this bug was raised to go
through the PostScript interpreter looking for places where we exit the
'C' level and return control to PostScript. This is done when we need
to evaluate something in the PostScript environment, such as a transfer
function or a tint transform.

Because these functions are written in PostScript we need to run them
in the PostScript environment.

To do this we push the procedure (or at least 'a' procedure) onto the
exec stack and exit with an o_push_estack error. In many cases that's
all we need to do, but sometimes we want to return control back to the
'C' environment and, in some of those cases, we want to store some state
for the C code. We can't use the operand stack (because the PostScript
function will alter that) so we store stuff on the exec stack instead.

When we complete the C level, we should restore the exec stack, so if
we stored any state on it, we should remove it. Sometimes we were not
doing so if there was an error.

Generally this did not cause a problem, because in general on an error
we would stop. However if the error handler had been altered it was
possible we might carry on. 'Sometimes' that would mean we tried to
execute something which wasn't executable, and sometimes it might mean
that we tried to return to the C level, but without the expected
state on the exec stack.

This could lead to memory corruption and crashes.

This commit tries to find everywhere where we might end up leaving
extra items on the exec stack in the case of an error, and either
removes the required number of items from the exec stack or uses
whatever cleanup routine was established for the C code.

Its important to note that, in normal use, none of these could actually
cause a problem. This makes it hard to test. all the cases here I have
tested, though in many cases the only way I could produce an error was
by forcing an error return in the debugger. I suspect some error cases
simply aren't possible but its good practice to check the return codes
anyway, even if its only a theoretical problem.

9 files changed, 86 insertions, 46 deletions

diff --git a/psi/zalg.c b/psi/zalg.c
index e2400d93d..2677e62c4 100644
--- a/psi/zalg.c
+++ b/psi/zalg.c
@@ -165,8 +165,10 @@ H6:	    H = 6;
             ref_assign(&op[0], &Rn[j]);
             break;
         case 6:
-/*H6_cont:*/if (!r_has_type(&op[0], t_boolean))
+            /*H6_cont:*/if (!r_has_type(&op[0], t_boolean)) {
+                esp -= 9;
                 return_error(gs_error_typecheck);
+            }
             if (op[0].value.boolval) {
 /* H7: */       ref_assign_old(&arry, &Rn[i], &Rn[j], ".sort(H7)");
                 goto H4;
@@ -176,6 +178,7 @@ H8:		ref_assign_old(&arry, &Rn[i], &R, ".sort(H8)");
             }
         default:
             pop(1);
+            esp -= 9;
             return_error(gs_error_unregistered); /* Must not happen. */
     }
     esp += 2;
diff --git a/psi/zcie.c b/psi/zcie.c
index c3c2cb4d0..efe2a2f63 100644
--- a/psi/zcie.c
+++ b/psi/zcie.c
@@ -455,25 +455,29 @@ ciedefgspace(i_ctx_t *i_ctx_p, ref *CIEDict, ulong dictkey)
     push(1); /* Sacrificial */
     procs = istate->colorspace[0].procs.cie;
     if (pcs == NULL ) {
-    if ((code = dict_find_string(CIEDict, "Table", &ptref)) <= 0)
-        return (code < 0 ? code : gs_note_error(gs_error_rangecheck));
-    check_read_type(*ptref, t_array);
-    if (r_size(ptref) != 5)
-        return_error(gs_error_rangecheck);
-        /* Stable memory due to current caching of color space */
-        code = gs_cspace_build_CIEDEFG(&pcs, NULL, mem->stable_memory);
-    if (code < 0)
-        return code;
-    pcie = pcs->params.defg;
-    pcie->Table.n = 4;
-    pcie->Table.m = 3;
+        if ((code = dict_find_string(CIEDict, "Table", &ptref)) <= 0) {
+            if (code == 0)
+                gs_note_error(cie_set_finish(i_ctx_p, pcs, &procs, edepth, gs_error_rangecheck));
+            else
+                return cie_set_finish(i_ctx_p, pcs, &procs, edepth, code);
+        }
+        check_read_type(*ptref, t_array);
+        if (r_size(ptref) != 5)
+            return_error(gs_error_rangecheck);
+            /* Stable memory due to current caching of color space */
+            code = gs_cspace_build_CIEDEFG(&pcs, NULL, mem->stable_memory);
+        if (code < 0)
+            return cie_set_finish(i_ctx_p, pcs, &procs, edepth, code);
+        pcie = pcs->params.defg;
+        pcie->Table.n = 4;
+        pcie->Table.m = 3;
         code = cie_cache_push_finish(i_ctx_p, cie_defg_finish, imem, pcie);
         if (code < 0)
-            return code;
+            return cie_set_finish(i_ctx_p, pcs, &procs, edepth, code);
         code = cie_defg_param(i_ctx_p, imemory, CIEDict, pcie, &procs,
             &has_abc_procs, &has_lmn_procs, &has_defg_procs,ptref);
         if (code < 0)
-            return code;
+            return cie_set_finish(i_ctx_p, pcs, &procs, edepth, code);
         /* Add the color space to the profile cache */
         gsicc_add_cs(igs, pcs,dictkey);
     } else {
@@ -561,25 +565,29 @@ ciedefspace(i_ctx_t *i_ctx_p, ref *CIEDict, ulong dictkey)
     push(1); /* Sacrificial */
     procs = istate->colorspace[0].procs.cie;
     if (pcs == NULL ) {
-    if ((code = dict_find_string(CIEDict, "Table", &ptref)) <= 0)
-        return (code < 0 ? code : gs_note_error(gs_error_rangecheck));
-    check_read_type(*ptref, t_array);
-    if (r_size(ptref) != 4)
-        return_error(gs_error_rangecheck);
-        /* Stable memory due to current caching of color space */
-        code = gs_cspace_build_CIEDEF(&pcs, NULL, mem->stable_memory);
-    if (code < 0)
-        return code;
-    pcie = pcs->params.def;
-    pcie->Table.n = 3;
-    pcie->Table.m = 3;
+        if ((code = dict_find_string(CIEDict, "Table", &ptref)) <= 0) {
+            if (code == 0)
+                gs_note_error(cie_set_finish(i_ctx_p, pcs, &procs, edepth, gs_error_rangecheck));
+            else
+                return cie_set_finish(i_ctx_p, pcs, &procs, edepth, code);
+        }
+        check_read_type(*ptref, t_array);
+        if (r_size(ptref) != 4)
+            return_error(gs_error_rangecheck);
+            /* Stable memory due to current caching of color space */
+            code = gs_cspace_build_CIEDEF(&pcs, NULL, mem->stable_memory);
+        if (code < 0)
+            return cie_set_finish(i_ctx_p, pcs, &procs, edepth, code);
+        pcie = pcs->params.def;
+        pcie->Table.n = 3;
+        pcie->Table.m = 3;
         code = cie_cache_push_finish(i_ctx_p, cie_def_finish, imem, pcie);
         if (code < 0)
-            return code;
+            return cie_set_finish(i_ctx_p, pcs, &procs, edepth, code);
         code = cie_def_param(i_ctx_p, imemory, CIEDict, pcie, &procs,
             &has_abc_procs, &has_lmn_procs, &has_def_procs, ptref);
         if (code < 0)
-            return code;
+            return cie_set_finish(i_ctx_p, pcs, &procs, edepth, code);
         /* Add the color space to the profile cache */
         gsicc_add_cs(igs, pcs,dictkey);
     } else {
@@ -629,15 +637,15 @@ cieabcspace(i_ctx_t *i_ctx_p, ref *CIEDict, ulong dictkey)
         /* Stable memory due to current caching of color space */
         code = gs_cspace_build_CIEABC(&pcs, NULL, mem->stable_memory);
     if (code < 0)
-        return code;
+        return cie_set_finish(i_ctx_p, pcs, &procs, edepth, code);
     pcie = pcs->params.abc;
         code = cie_cache_push_finish(i_ctx_p, cie_abc_finish, imem, pcie);
         if (code < 0)
-            return code;
+            return cie_set_finish(i_ctx_p, pcs, &procs, edepth, code);
         code = cie_abc_param(i_ctx_p, imemory, CIEDict, pcie, &procs,
             &has_abc_procs, &has_lmn_procs);
         if (code < 0)
-            return code;
+            return cie_set_finish(i_ctx_p, pcs, &procs, edepth, code);
         /* Set the color space in the graphic state.  The ICC profile
             will be set later if we actually use the space.  Procs will be
             sampled now though. Also, the finish procedure is on the stack
@@ -692,16 +700,16 @@ cieaspace(i_ctx_t *i_ctx_p, ref *CIEdict, ulong dictkey)
         /* Stable memory due to current caching of color space */
         code = gs_cspace_build_CIEA(&pcs, NULL, mem->stable_memory);
     if (code < 0)
-        return code;
+        return cie_set_finish(i_ctx_p, pcs, &procs, edepth, code);
     pcie = pcs->params.a;
         code = cie_a_param(imemory, CIEdict, pcie, &procs, &has_a_procs,
                                 &has_lmn_procs);
         if (code < 0)
-            return code;
+            return cie_set_finish(i_ctx_p, pcs, &procs, edepth, code);
         /* Push finalize procedure on the execution stack */
         code = cie_cache_push_finish(i_ctx_p, cie_a_finish, (gs_ref_memory_t *)imem, pcie);
         if (code < 0)
-            return code;
+            return cie_set_finish(i_ctx_p, pcs, &procs, edepth, code);
         if (!has_a_procs && !has_lmn_procs) {
             pcie->common.caches.DecodeLMN->floats
                 .params.is_identity = true;
@@ -713,7 +721,7 @@ cieaspace(i_ctx_t *i_ctx_p, ref *CIEdict, ulong dictkey)
                 code = cie_prepare_iccproc(i_ctx_p, &pcie->RangeA,
                     &procs.Decode.A, &pcie->caches.DecodeA.floats, pcie, imem, "Decode.A");
                 if (code < 0)
-                    return code;
+                    return cie_set_finish(i_ctx_p, pcs, &procs, edepth, code);
             } else {
                 pcie->caches.DecodeA.floats.params.is_identity = true;
             }
@@ -842,8 +850,10 @@ cie_cache_finish_store(i_ctx_t *i_ctx_p, bool replicate)
             code = float_param(ref_stack_index(&o_stack,
                                (replicate ? 0 : gx_cie_cache_size - 1 - i)),
                                &pcache->values[i]);
-            if (code < 0)
+            if (code < 0) {
+                esp -= 2;                       /* pop pointer to cache */
                 return code;
+            }
         }
     }
 #ifdef DEBUG
diff --git a/psi/zcolor1.c b/psi/zcolor1.c
index 56dcf71d8..1a341ff41 100644
--- a/psi/zcolor1.c
+++ b/psi/zcolor1.c
@@ -94,6 +94,7 @@ static int
 zsetcolortransfer(i_ctx_t *i_ctx_p)
 {
     os_ptr op = osp;
+    os_ptr ep = esp;
     int code;
 
     check_proc(op[-3]);
@@ -130,8 +131,14 @@ zsetcolortransfer(i_ctx_t *i_ctx_p)
         (code = zcolor_remap_one(i_ctx_p, &istate->transfer_procs.gray,
                                  igs->set_transfer.gray, igs,
                                  zcolor_remap_one_finish)) < 0
-        )
+                                 )
+    {
+        /* Return the exec stack to the state it was in before we started the setup
+         * for the transfer function evaluation.
+         */
+        esp = ep;
         return code;
+    }
     return o_push_estack;
 }
 
diff --git a/psi/zcontrol.c b/psi/zcontrol.c
index ebcd5e385..36da22ca0 100644
--- a/psi/zcontrol.c
+++ b/psi/zcontrol.c
@@ -239,8 +239,10 @@ end_runandhide(i_ctx_t *i_ctx_p)
 {
     int code;
 
-    if ((code = runandhide_restore_hidden(i_ctx_p, esp, esp - 1)) < 0)
+    if ((code = runandhide_restore_hidden(i_ctx_p, esp, esp - 1)) < 0) {
+        esp -= 2;
         return code;
+    }
     esp -= 2;           /* pop the hidden value and its atributes */
     return o_pop_estack;
 }
diff --git a/psi/zfile.c b/psi/zfile.c
index b30784029..d6575ea86 100644
--- a/psi/zfile.c
+++ b/psi/zfile.c
@@ -425,8 +425,10 @@ file_continue(i_ctx_t *i_ctx_p)
     uint len = r_size(pscratch);
     uint code;
 
-    if (len < devlen)
+    if (len < devlen) {
+        esp -= 5;               /* pop proc, pfen, devlen, iodev , mark */
         return_error(gs_error_rangecheck);     /* not even room for device len */
+    }
 
     do {
         memcpy((char *)pscratch->value.bytes, iodev->dname, devlen);
@@ -435,8 +437,10 @@ file_continue(i_ctx_t *i_ctx_p)
         if (code == ~(uint) 0) {    /* all done */
             esp -= 5;               /* pop proc, pfen, devlen, iodev , mark */
             return o_pop_estack;
-        } else if (code > len)      /* overran string */
+        } else if (code > len) {      /* overran string */
+            esp -= 5;               /* pop proc, pfen, devlen, iodev , mark */
             return_error(gs_error_rangecheck);
+        }
         else if (iodev != iodev_default(imemory)
               || (check_file_permissions(i_ctx_p, (char *)pscratch->value.bytes, code + devlen, iodev, "PermitFileReading")) == 0) {
             push(1);
diff --git a/psi/zht1.c b/psi/zht1.c
index 86f32e858..7265abbcf 100644
--- a/psi/zht1.c
+++ b/psi/zht1.c
@@ -115,8 +115,11 @@ setcolorscreen_finish(i_ctx_t *i_ctx_p)
 
     pdht->order = pdht->components[0].corder;
     code = gx_ht_install(igs, r_ptr(esp - 1, gs_halftone), pdht);
-    if (code < 0)
+    if (code < 0) {
+        esp -= 7;
+        setcolorscreen_cleanup(i_ctx_p);
         return code;
+    }
     istate->screen_procs.red   = esp[-5];
     istate->screen_procs.green = esp[-4];
     istate->screen_procs.blue  = esp[-3];
diff --git a/psi/zht2.c b/psi/zht2.c
index 48d7dd3b8..60bd6567e 100644
--- a/psi/zht2.c
+++ b/psi/zht2.c
@@ -558,8 +558,11 @@ sethalftone_finish(i_ctx_t *i_ctx_p)
     if (pdht->components)
         pdht->order = pdht->components[0].corder;
     code = gx_ht_install(igs, r_ptr(esp - 1, gs_halftone), pdht);
-    if (code < 0)
+    if (code < 0) {
+        esp -= 4;
+        sethalftone_cleanup(i_ctx_p);
         return code;
+    }
     istate->halftone = esp[-2];
     esp -= 4;
     sethalftone_cleanup(i_ctx_p);
diff --git a/psi/zpath1.c b/psi/zpath1.c
index e9eca1c44..b9fb2e593 100644
--- a/psi/zpath1.c
+++ b/psi/zpath1.c
@@ -232,6 +232,8 @@ path_continue(i_ctx_t *i_ctx_p)
             path_cleanup(i_ctx_p);
             return o_pop_estack;
         default:                /* error */
+            esp -= 6;
+            path_cleanup(i_ctx_p);
             return code;
         case gs_pe_moveto:
             esp[2] = esp[-4];   /* moveto proc */
diff --git a/psi/zpcolor.c b/psi/zpcolor.c
index fb601a303..3c3e29e21 100644
--- a/psi/zpcolor.c
+++ b/psi/zpcolor.c
@@ -351,8 +351,10 @@ pattern_paint_finish(i_ctx_t *i_ctx_p)
             }
             pinst = (gs_pattern1_instance_t *)gs_currentcolor(igs->saved)->pattern;
             /* If pinst is NULL after all of that then we are not going to recover */
-            if (pinst == NULL)
+            if (pinst == NULL) {
+                esp -= 5;
                 return_error(gs_error_unknownerror);
+            }
         }
         pgs = igs;
 
@@ -360,8 +362,10 @@ pattern_paint_finish(i_ctx_t *i_ctx_p)
             if (pinst->is_clist) {
                 /* Send the compositor command to close the PDF14 device */
                 code = gs_pop_pdf14trans_device(pgs, true);
-                if (code < 0)
+                if (code < 0) {
+                    esp -= 5;
                     return code;
+                }
             } else {
                 /* Not a clist, get PDF14 buffer information */
                 code = pdf14_get_buffer_information(pgs->device,
@@ -369,8 +373,10 @@ pattern_paint_finish(i_ctx_t *i_ctx_p)
                                                     true);
                 /* PDF14 device (and buffer) is destroyed when pattern cache
                    entry is removed */
-                if (code < 0)
+                if (code < 0) {
+                    esp -= 5;
                     return code;
+                }
             }
         }
         code = gx_pattern_cache_add_entry(igs, pdev, &ctile);