From da3382e9893f1ff48e0fc358ecea8cf9025ce0b1 Mon Sep 17 00:00:00 2001 From: "David Kaspar [Dee'Kej]" Date: Wed, 29 Aug 2018 12:58:23 +0200 Subject: [PATCH] fixes-for-set-of-CVEs-reported-by-Google.patch added According to upstream, this should deal with the issues reported here: http://seclists.org/oss-sec/2018/q3/142 Although, it's possible some follow-up patches will be needed as well. --- ...s-for-set-of-CVEs-reported-by-Google.patch | 854 ++++++++++++++++++ ghostscript.spec | 6 +- 2 files changed, 859 insertions(+), 1 deletion(-) create mode 100644 ghostscript-9.23-002-fixes-for-set-of-CVEs-reported-by-Google.patch diff --git a/ghostscript-9.23-002-fixes-for-set-of-CVEs-reported-by-Google.patch b/ghostscript-9.23-002-fixes-for-set-of-CVEs-reported-by-Google.patch new file mode 100644 index 0000000..75dc6ca --- /dev/null +++ b/ghostscript-9.23-002-fixes-for-set-of-CVEs-reported-by-Google.patch @@ -0,0 +1,854 @@ +From 23499be58677663ba69d2b29618bf05c4366aa34 Mon Sep 17 00:00:00 2001 +From: Chris Liddell +Date: Thu, 23 Aug 2018 12:20:56 +0100 +Subject: [PATCH 01/13] Bug 699668: handle stack overflow during error handling + +When handling a Postscript error, we push the object throwing the error onto +the operand stack for the error handling procedure to access - we were not +checking the available stack before doing so, thus causing a crash. + +Basically, if we get a stack overflow when already handling an error, we're out +of options, return to the caller with a fatal error. +--- + psi/interp.c | 7 ++++++- + 1 file changed, 6 insertions(+), 1 deletion(-) + +diff --git a/psi/interp.c b/psi/interp.c +index 8b49556..6150838 100644 +--- a/psi/interp.c ++++ b/psi/interp.c +@@ -676,7 +676,12 @@ again: + /* Push the error object on the operand stack if appropriate. */ + if (!GS_ERROR_IS_INTERRUPT(code)) { + /* Replace the error object if within an oparray or .errorexec. */ +- *++osp = *perror_object; ++ osp++; ++ if (osp >= ostop) { ++ *pexit_code = gs_error_Fatal; ++ return_error(gs_error_Fatal); ++ } ++ *osp = *perror_object; + errorexec_find(i_ctx_p, osp); + } + goto again; +-- +2.14.4 + + +From 3f5427b4ee15054127a9127bfbc6e1b1371992ca Mon Sep 17 00:00:00 2001 +From: Ken Sharp +Date: Thu, 23 Aug 2018 15:42:02 +0100 +Subject: [PATCH 02/13] Bug 699665 "memory corruption in aesdecode" + +The specimen file calls aesdecode without specifying the key to be +used, though it does manage to do enough work with the PDF interpreter +routines to get access to aesdecode (which isn't normally available). + +This causes us to read uninitialised memory, which can (and often does) +lead to a segmentation fault. + +In this commit we set the key to NULL explicitly during intialisation +and then check it before we read it. If its NULL we just return. + +It seems bizarre that we don't return error codes, we should probably +look into that at some point, but this prevents the code trying to +read uninitialised memory. +--- + base/aes.c | 3 +++ + base/saes.c | 1 + + 2 files changed, 4 insertions(+) + +diff --git a/base/aes.c b/base/aes.c +index a6bce93..e86f000 100644 +--- a/base/aes.c ++++ b/base/aes.c +@@ -662,6 +662,9 @@ void aes_crypt_ecb( aes_context *ctx, + } + #endif + ++ if (ctx == NULL || ctx->rk == NULL) ++ return; ++ + RK = ctx->rk; + + GET_ULONG_LE( X0, input, 0 ); X0 ^= *RK++; +diff --git a/base/saes.c b/base/saes.c +index 6db0e8b..307ed74 100644 +--- a/base/saes.c ++++ b/base/saes.c +@@ -120,6 +120,7 @@ s_aes_process(stream_state * ss, stream_cursor_read * pr, + gs_throw(gs_error_VMerror, "could not allocate aes context"); + return ERRC; + } ++ memset(state->ctx, 0x00, sizeof(aes_context)); + if (state->keylength < 1 || state->keylength > SAES_MAX_KEYLENGTH) { + gs_throw1(gs_error_rangecheck, "invalid aes key length (%d bytes)", + state->keylength); +-- +2.14.4 + + +From b47d8dc0bdea68a2b05ece2b1ee05becb9667ee9 Mon Sep 17 00:00:00 2001 +From: Chris Liddell +Date: Thu, 23 Aug 2018 15:41:18 +0100 +Subject: [PATCH 03/13] Bug 699664: Ensure the correct is in place before + cleanup + +If the PS job replaces the device and leaves that graphics state in place, we +wouldn't cleanup the default device in the normal way, but rely on the garbage +collector. + +This works (but isn't ideal), *except* when the job replaces the device with +the null device (using the nulldevice operator) - this means that +.uninstallpagedevice doesn't replace the existing device with the nulldevice +(since it is already installed), the device from the graphics ends up being +freed - and as it is the nulldevice, which we rely on, memory corruption +and a segfault can happen. + +We avoid this by checking if the current device is the nulldevice, and if so, +restoring it away, before continuing with the device cleanup. +--- + psi/imain.c | 10 ++++++++++ + 1 file changed, 10 insertions(+) + +diff --git a/psi/imain.c b/psi/imain.c +index 2fe1546..138bfc8 100644 +--- a/psi/imain.c ++++ b/psi/imain.c +@@ -936,6 +936,16 @@ gs_main_finit(gs_main_instance * minst, int exit_status, int code) + i_ctx_p = minst->i_ctx_p; /* interp_reclaim could change it. */ + } + ++ if (i_ctx_p->pgs != NULL && i_ctx_p->pgs->device != NULL && ++ gx_device_is_null(i_ctx_p->pgs->device)) { ++ /* if the job replaced the device with the nulldevice, we we need to grestore ++ away that device, so the block below can properly dispense ++ with the default device. ++ */ ++ int code = gs_grestoreall(i_ctx_p->pgs); ++ if (code < 0) return_error(gs_error_Fatal); ++ } ++ + if (i_ctx_p->pgs != NULL && i_ctx_p->pgs->device != NULL) { + gx_device *pdev = i_ctx_p->pgs->device; + const char * dname = pdev->dname; +-- +2.14.4 + + +From cd1caf1f63f645260481a73d9990ec28cf9ba79b Mon Sep 17 00:00:00 2001 +From: Chris Liddell +Date: Thu, 23 Aug 2018 14:13:25 +0100 +Subject: [PATCH 04/13] Bug 699661: Avoid sharing pointers between pdf14 + compositors + +If a copdevice is triggered when the pdf14 compositor is the device, we make +a copy of the device, then throw an error because, by default we're only allowed +to copy the device prototype - then freeing it calls the finalize, which frees +several pointers shared with the parent. + +Make a pdf14 specific finish_copydevice() which NULLs the relevant pointers, +before, possibly, throwing the same error as the default method. + +This also highlighted a problem with reopening the X11 devices, where a custom +error handler could be replaced with itself, meaning it also called itself, +and infifite recursion resulted. + +Keep a note of if the handler replacement has been done, and don't do it a +second time. +--- + base/gdevp14.c | 17 ++++++++++++++++- + devices/gdevxini.c | 12 ++++++++---- + 2 files changed, 24 insertions(+), 5 deletions(-) + +diff --git a/base/gdevp14.c b/base/gdevp14.c +index d972fc2..7cf81b2 100644 +--- a/base/gdevp14.c ++++ b/base/gdevp14.c +@@ -178,6 +178,7 @@ static dev_proc_fill_mask(pdf14_fill_mask); + static dev_proc_stroke_path(pdf14_stroke_path); + static dev_proc_begin_typed_image(pdf14_begin_typed_image); + static dev_proc_text_begin(pdf14_text_begin); ++static dev_proc_finish_copydevice(pdf14_finish_copydevice); + static dev_proc_create_compositor(pdf14_create_compositor); + static dev_proc_create_compositor(pdf14_forward_create_compositor); + static dev_proc_begin_transparency_group(pdf14_begin_transparency_group); +@@ -245,7 +246,7 @@ static const gx_color_map_procs * + pdf14_create_compositor, /* create_compositor */\ + NULL, /* get_hardware_params */\ + pdf14_text_begin, /* text_begin */\ +- NULL, /* finish_copydevice */\ ++ pdf14_finish_copydevice, /* finish_copydevice */\ + pdf14_begin_transparency_group,\ + pdf14_end_transparency_group,\ + pdf14_begin_transparency_mask,\ +@@ -3952,6 +3953,19 @@ pdf14_text_begin(gx_device * dev, gs_gstate * pgs, + return code; + } + ++static int ++pdf14_finish_copydevice(gx_device *new_dev, const gx_device *from_dev) ++{ ++ pdf14_device *pdev = (pdf14_device*)new_dev; ++ ++ pdev->ctx = NULL; ++ pdev->trans_group_parent_cmap_procs = NULL; ++ pdev->smaskcolor = NULL; ++ ++ /* Only allow copying the prototype. */ ++ return (from_dev->memory ? gs_note_error(gs_error_rangecheck) : 0); ++} ++ + /* + * Implement copy_mono by filling lots of small rectangles. + */ +@@ -8113,6 +8127,7 @@ c_pdf14trans_clist_read_update(gs_composite_t * pcte, gx_device * cdev, + before reopening the device */ + if (p14dev->ctx != NULL) { + pdf14_ctx_free(p14dev->ctx); ++ p14dev->ctx = NULL; + } + dev_proc(tdev, open_device) (tdev); + } +diff --git a/devices/gdevxini.c b/devices/gdevxini.c +index 8511eac..23b8c35 100644 +--- a/devices/gdevxini.c ++++ b/devices/gdevxini.c +@@ -59,7 +59,8 @@ static struct xv_ { + Boolean alloc_error; + XErrorHandler orighandler; + XErrorHandler oldhandler; +-} x_error_handler; ++ Boolean set; ++} x_error_handler = {0}; + + static int + x_catch_alloc(Display * dpy, XErrorEvent * err) +@@ -74,7 +75,8 @@ x_catch_alloc(Display * dpy, XErrorEvent * err) + int + x_catch_free_colors(Display * dpy, XErrorEvent * err) + { +- if (err->request_code == X_FreeColors) ++ if (err->request_code == X_FreeColors || ++ x_error_handler.orighandler == x_catch_free_colors) + return 0; + return x_error_handler.orighandler(dpy, err); + } +@@ -274,8 +276,10 @@ gdev_x_open(gx_device_X * xdev) + return_error(gs_error_ioerror); + } + /* Buggy X servers may cause a Bad Access on XFreeColors. */ +- x_error_handler.orighandler = XSetErrorHandler(x_catch_free_colors); +- ++ if (!x_error_handler.set) { ++ x_error_handler.orighandler = XSetErrorHandler(x_catch_free_colors); ++ x_error_handler.set = True; ++ } + /* Get X Resources. Use the toolkit for this. */ + XtToolkitInitialize(); + app_con = XtCreateApplicationContext(); +-- +2.14.4 + + +From a6b4cbf5def8a61013f2ef787d6618328cdf92ef Mon Sep 17 00:00:00 2001 +From: Ken Sharp +Date: Thu, 23 Aug 2018 14:12:48 +0100 +Subject: [PATCH 05/13] Fix Bug 699660 "shading_param incomplete type checking" + +Its possible to pass a t_struct parameter to .shfill which is not a +shading function built by .buildshading. This could then lead to memory +corruption or a segmentation fault by treating the object passed in +as if it were a shading. + +Its non-trivial to check the t_struct, because this function can take +7 different kinds of structures as a parameter. Checking these is +possible, of course, but would add a performance penalty. + +However, we can note that we never call .shfill without first calling +.buildshading, and we never call .buildshading without immediately +calling .shfill. So we can treat these as an atomic operation. The +.buildshading function takes all its parameters as PostScript objects +and validates them, so that should be safe. + +This allows us to 'hide' the .shfill operator preventing the possibility +of passing an invalid parameter. +--- + Resource/Init/gs_init.ps | 4 ++-- + Resource/Init/gs_ll3.ps | 7 ++++++- + Resource/Init/pdf_draw.ps | 3 +-- + 3 files changed, 9 insertions(+), 5 deletions(-) + +diff --git a/Resource/Init/gs_init.ps b/Resource/Init/gs_init.ps +index 6c8da53..1956ed5 100644 +--- a/Resource/Init/gs_init.ps ++++ b/Resource/Init/gs_init.ps +@@ -2181,8 +2181,8 @@ SAFER { .setsafeglobal } if + /.getiodevice /.getdevparms /.putdevparams /.bbox_transform /.matchmedia /.matchpagesize /.defaultpapersize + /.oserrno /.setoserrno /.oserrorstring /.getCPSImode + /.getscanconverter /.setscanconverter /.type1encrypt /.type1decrypt/.languagelevel /.setlanguagelevel /.eqproc /.fillpage /.buildpattern1 /.saslprep +-/.buildshading1 /.buildshadin2 /.buildshading3 /.buildshading4 /.buildshading5 /.buildshading6 /.buildshading7 /.buildshadingpattern +-/.argindex /.bytestring /.namestring /.stringbreak /.stringmatch /.globalvmarray /.globalvmdict /.globalvmpackedarray /.globalvmstring ++/.buildshading1 /.buildshading2 /.buildshading3 /.buildshading4 /.buildshading5 /.buildshading6 /.buildshading7 /.buildshadingpattern ++%/.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 +diff --git a/Resource/Init/gs_ll3.ps b/Resource/Init/gs_ll3.ps +index 5aa56a3..1d37e53 100644 +--- a/Resource/Init/gs_ll3.ps ++++ b/Resource/Init/gs_ll3.ps +@@ -440,6 +440,11 @@ systemdict /.reuseparamdict mark + /shfill .systemvar /undefined signalerror + } ifelse + } bind def ++ ++/.buildshading_and_shfill { ++ .buildshading .shfill ++} bind def ++ + systemdict /.reuseparamdict undef + + /.buildpattern2 { %