Bug 699695: Remember to end groups/softmasks even upon exception.

fz_fill_path() may throw an exception halfway through
pdf_show_path(), which in this case would not attempt to end any
begun groups or softmasks. This led to e.g. leaks of pixmaps held
by a group that was never ended.

Moving the cleanup to the always block is not foolproof because
the cleanup code itself may also throw exceptions, hence
preventing the end of the fz_always block from being executed.
This commit does put pdf_show_path() in the same situation as
pdf_run_xobject() that has the same problem with its cleanup
code.

Thanks to oss-fuzz for reporting.

1 files changed, 54 insertions, 9 deletions

diff --git a/source/pdf/pdf-op-run.c b/source/pdf/pdf-op-run.c
index 1ad0e610..5892dcb8 100644
--- a/source/pdf/pdf-op-run.c
+++ b/source/pdf/pdf-op-run.c
@@ -607,6 +607,12 @@ pdf_show_path(fz_context *ctx, pdf_run_processor *pr, int doclose, int dofill, i
 	fz_rect bbox;
 	softmask_save softmask = { NULL };
 	int knockout_group = 0;
+	char errmess[256] = "";
+	int err = FZ_ERROR_NONE;
+
+	fz_var(knockout_group);
+	fz_var(bbox);
+	fz_var(path);
 
 	if (dostroke) {
 		if (pr->dev->flags & (FZ_DEVFLAG_STROKECOLOR_UNDEFINED | FZ_DEVFLAG_LINEJOIN_UNDEFINED | FZ_DEVFLAG_LINEWIDTH_UNDEFINED))
@@ -713,28 +719,67 @@ pdf_show_path(fz_context *ctx, pdf_run_processor *pr, int doclose, int dofill, i
 				break;
 			}
 		}
-
+	}
+	fz_always(ctx)
+	{
 		if (knockout_group)
-			fz_end_group(ctx, pr->dev);
+		{
+			fz_try(ctx)
+			{
+				fz_end_group(ctx, pr->dev);
+			}
+			fz_catch(ctx)
+			{
+				/* Postpone the problem */
+				err = fz_caught(ctx);
+				strcpy(errmess, fz_caught_message(ctx));
+			}
+		}
 
 		if (dofill || dostroke)
-			pdf_end_group(ctx, pr, &softmask);
+		{
+			fz_try(ctx)
+			{
+				pdf_end_group(ctx, pr, &softmask);
+			}
+			fz_catch(ctx)
+			{
+				/* Postpone the problem */
+				if (err)
+					fz_warn(ctx, "ignoring error: %s", fz_caught_message(ctx));
+				err = fz_caught(ctx);
+				strcpy(errmess, fz_caught_message(ctx));
+			}
+		}
 
 		if (pr->clip)
 		{
-			gstate->clip_depth++;
-			fz_clip_path(ctx, pr->dev, path, pr->clip_even_odd, gstate->ctm, bbox);
-			pr->clip = 0;
+			fz_try(ctx)
+			{
+				gstate->clip_depth++;
+				fz_clip_path(ctx, pr->dev, path, pr->clip_even_odd, gstate->ctm, bbox);
+				pr->clip = 0;
+			}
+			fz_catch(ctx)
+			{
+				/* Postpone the problem */
+				if (err)
+					fz_warn(ctx, "ignoring error: %s", fz_caught_message(ctx));
+				err = fz_caught(ctx);
+				strcpy(errmess, fz_caught_message(ctx));
+			}
 		}
-	}
-	fz_always(ctx)
-	{
+
 		fz_drop_path(ctx, path);
 	}
 	fz_catch(ctx)
 	{
 		fz_rethrow(ctx);
 	}
+
+	/* Rethrow postponed errors */
+	if (err)
+		fz_throw(ctx, err, "%s", errmess);
 }
 
 /*