unofficial mirror of guix-devel@gnu.org 
 help / color / mirror / code / Atom feed
* [PATCH] gnu: mupdf: Fix some security problems in bundled mujs.
@ 2017-01-12 18:06 Marius Bakke
  2017-01-12 18:30 ` Leo Famulari
  0 siblings, 1 reply; 13+ messages in thread
From: Marius Bakke @ 2017-01-12 18:06 UTC (permalink / raw)
  To: guix-devel; +Cc: Marius Bakke

* gnu/packages/patches/mupdf-mujs-heap-buffer-overflow.patch: New file.
* gnu/packages/patches/mupdf-mujs-null-pointer-dereference.patch: New file.
* gnu/local.mk (dist_patch_DATA): Add them.
* gnu/packages/pdf.scm (mupdf)[source]: Use them.
---
 gnu/local.mk                                       |   2 +
 .../patches/mupdf-mujs-heap-buffer-overflow.patch  |  34 ++++
 .../mupdf-mujs-null-pointer-dereference.patch      | 187 +++++++++++++++++++++
 gnu/packages/pdf.scm                               |   4 +-
 4 files changed, 226 insertions(+), 1 deletion(-)
 create mode 100644 gnu/packages/patches/mupdf-mujs-heap-buffer-overflow.patch
 create mode 100644 gnu/packages/patches/mupdf-mujs-null-pointer-dereference.patch

diff --git a/gnu/local.mk b/gnu/local.mk
index 21580a387..365513107 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -753,6 +753,8 @@ dist_patch_DATA =						\
   %D%/packages/patches/multiqc-fix-git-subprocess-error.patch	\
   %D%/packages/patches/mumps-build-parallelism.patch		\
   %D%/packages/patches/mupdf-build-with-openjpeg-2.1.patch	\
+  %D%/packages/patches/mupdf-mujs-heap-buffer-overflow.patch	\
+  %D%/packages/patches/mupdf-mujs-null-pointer-dereference.patch	\
   %D%/packages/patches/mupen64plus-ui-console-notice.patch	\
   %D%/packages/patches/musl-CVE-2016-8859.patch			\
   %D%/packages/patches/mutt-store-references.patch		\
diff --git a/gnu/packages/patches/mupdf-mujs-heap-buffer-overflow.patch b/gnu/packages/patches/mupdf-mujs-heap-buffer-overflow.patch
new file mode 100644
index 000000000..77a3e45cd
--- /dev/null
+++ b/gnu/packages/patches/mupdf-mujs-heap-buffer-overflow.patch
@@ -0,0 +1,34 @@
+This fixes a heap buffer overflow in MuJS as reported at
+
+http://seclists.org/oss-sec/2017/q1/74
+
+Patch lifted from upstream source repository:
+
+https://git.ghostscript.com/?p=mujs.git;h=77ab465f1c394bb77f00966cd950650f3f53cb24
+
+From 77ab465f1c394bb77f00966cd950650f3f53cb24 Mon Sep 17 00:00:00 2001
+From: Tor Andersson <tor.andersson@gmail.com>
+Date: Thu, 12 Jan 2017 14:47:01 +0100
+Subject: [PATCH] Fix 697401: Error when dropping extra arguments to
+ lightweight functions.
+
+---
+ thirdparty/mujs/jsrun.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/thirdparty/mujs/jsrun.c b/thirdparty/mujs/jsrun.c
+index ee80845..782a6f9 100644
+--- a/thirdparty/mujs/jsrun.c
++++ b/thirdparty/mujs/jsrun.c
+@@ -937,7 +937,7 @@ static void jsR_calllwfunction(js_State *J, int n, js_Function *F, js_Environmen
+ 	jsR_savescope(J, scope);
+ 
+ 	if (n > F->numparams) {
+-		js_pop(J, F->numparams - n);
++		js_pop(J, n - F->numparams);
+ 		n = F->numparams;
+ 	}
+ 	for (i = n; i < F->varlen; ++i)
+-- 
+2.9.1
+
diff --git a/gnu/packages/patches/mupdf-mujs-null-pointer-dereference.patch b/gnu/packages/patches/mupdf-mujs-null-pointer-dereference.patch
new file mode 100644
index 000000000..9fe1921ef
--- /dev/null
+++ b/gnu/packages/patches/mupdf-mujs-null-pointer-dereference.patch
@@ -0,0 +1,187 @@
+This fixes a null pointer dereference in MuJS regexp.c as
+reported at:
+
+http://seclists.org/oss-sec/2017/q1/74
+
+Patch lifted from upstream source repository:
+
+http://git.ghostscript.com/?p=mujs.git;h=fd003eceda531e13fbdd1aeb6e9c73156496e569
+
+From fd003eceda531e13fbdd1aeb6e9c73156496e569 Mon Sep 17 00:00:00 2001
+From: Tor Andersson <tor@ccxvii.net>
+Date: Fri, 2 Dec 2016 14:56:20 -0500
+Subject: [PATCH] Fix 697381: check allocation when compiling regular
+ expressions.
+
+Also use allocator callback function.
+---
+ thirdparty/mujs/jsgc.c     |  2 +-
+ thirdparty/mujs/jsregexp.c |  2 +-
+ thirdparty/mujs/jsstate.c  |  6 ------
+ thirdparty/mujs/regexp.c   | 45 +++++++++++++++++++++++++++++++++++----------
+ thirdparty/mujs/regexp.h   |  7 +++++++
+ 5 files changed, 44 insertions(+), 18 deletions(-)
+
+diff --git a/thirdparty/mujs/jsgc.c b/thirdparty/mujs/jsgc.c
+index 4f7e7dc..f80111e 100644
+--- a/thirdparty/mujs/jsgc.c
++++ b/thirdparty/mujs/jsgc.c
+@@ -46,7 +46,7 @@ static void jsG_freeobject(js_State *J, js_Object *obj)
+ 		jsG_freeproperty(J, obj->head);
+ 	if (obj->type == JS_CREGEXP) {
+ 		js_free(J, obj->u.r.source);
+-		js_regfree(obj->u.r.prog);
++		js_regfreex(J->alloc, J->actx, obj->u.r.prog);
+ 	}
+ 	if (obj->type == JS_CITERATOR)
+ 		jsG_freeiterator(J, obj->u.iter.head);
+diff --git a/thirdparty/mujs/jsregexp.c b/thirdparty/mujs/jsregexp.c
+index a2d5156..7b09c06 100644
+--- a/thirdparty/mujs/jsregexp.c
++++ b/thirdparty/mujs/jsregexp.c
+@@ -16,7 +16,7 @@ void js_newregexp(js_State *J, const char *pattern, int flags)
+ 	if (flags & JS_REGEXP_I) opts |= REG_ICASE;
+ 	if (flags & JS_REGEXP_M) opts |= REG_NEWLINE;
+ 
+-	prog = js_regcomp(pattern, opts, &error);
++	prog = js_regcompx(J->alloc, J->actx, pattern, opts, &error);
+ 	if (!prog)
+ 		js_syntaxerror(J, "regular expression: %s", error);
+ 
+diff --git a/thirdparty/mujs/jsstate.c b/thirdparty/mujs/jsstate.c
+index 638cab3..fd5bcf6 100644
+--- a/thirdparty/mujs/jsstate.c
++++ b/thirdparty/mujs/jsstate.c
+@@ -9,12 +9,6 @@
+ 
+ static void *js_defaultalloc(void *actx, void *ptr, int size)
+ {
+-	if (size == 0) {
+-		free(ptr);
+-		return NULL;
+-	}
+-	if (!ptr)
+-		return malloc((size_t)size);
+ 	return realloc(ptr, (size_t)size);
+ }
+ 
+diff --git a/thirdparty/mujs/regexp.c b/thirdparty/mujs/regexp.c
+index 9852be2..01c18a3 100644
+--- a/thirdparty/mujs/regexp.c
++++ b/thirdparty/mujs/regexp.c
+@@ -807,23 +807,31 @@ static void dumpprog(Reprog *prog)
+ }
+ #endif
+ 
+-Reprog *regcomp(const char *pattern, int cflags, const char **errorp)
++Reprog *regcompx(void *(*alloc)(void *ctx, void *p, int n), void *ctx,
++	const char *pattern, int cflags, const char **errorp)
+ {
+ 	struct cstate g;
+ 	Renode *node;
+ 	Reinst *split, *jump;
+ 	int i;
+ 
+-	g.prog = malloc(sizeof (Reprog));
+-	g.pstart = g.pend = malloc(sizeof (Renode) * strlen(pattern) * 2);
++	g.pstart = NULL;
++	g.prog = NULL;
+ 
+ 	if (setjmp(g.kaboom)) {
+ 		if (errorp) *errorp = g.error;
+-		free(g.pstart);
+-		free(g.prog);
++		alloc(ctx, g.pstart, 0);
++		alloc(ctx, g.prog, 0);
+ 		return NULL;
+ 	}
+ 
++	g.prog = alloc(ctx, NULL, sizeof (Reprog));
++	if (!g.prog)
++		die(&g, "cannot allocate regular expression");
++	g.pstart = g.pend = alloc(ctx, NULL, sizeof (Renode) * strlen(pattern) * 2);
++	if (!g.pstart)
++		die(&g, "cannot allocate regular expression parse list");
++
+ 	g.source = pattern;
+ 	g.ncclass = 0;
+ 	g.nsub = 1;
+@@ -840,7 +848,9 @@ Reprog *regcomp(const char *pattern, int cflags, const char **errorp)
+ 		die(&g, "syntax error");
+ 
+ 	g.prog->nsub = g.nsub;
+-	g.prog->start = g.prog->end = malloc((count(node) + 6) * sizeof (Reinst));
++	g.prog->start = g.prog->end = alloc(ctx, NULL, (count(node) + 6) * sizeof (Reinst));
++	if (!g.prog->start)
++		die(&g, "cannot allocate regular expression instruction list");
+ 
+ 	split = emit(g.prog, I_SPLIT);
+ 	split->x = split + 3;
+@@ -859,20 +869,35 @@ Reprog *regcomp(const char *pattern, int cflags, const char **errorp)
+ 	dumpprog(g.prog);
+ #endif
+ 
+-	free(g.pstart);
++	alloc(ctx, g.pstart, 0);
+ 
+ 	if (errorp) *errorp = NULL;
+ 	return g.prog;
+ }
+ 
+-void regfree(Reprog *prog)
++void regfreex(void *(*alloc)(void *ctx, void *p, int n), void *ctx, Reprog *prog)
+ {
+ 	if (prog) {
+-		free(prog->start);
+-		free(prog);
++		alloc(ctx, prog->start, 0);
++		alloc(ctx, prog, 0);
+ 	}
+ }
+ 
++static void *default_alloc(void *ctx, void *p, int n)
++{
++	return realloc(p, (size_t)n);
++}
++
++Reprog *regcomp(const char *pattern, int cflags, const char **errorp)
++{
++	return regcompx(default_alloc, NULL, pattern, cflags, errorp);
++}
++
++void regfree(Reprog *prog)
++{
++	regfreex(default_alloc, NULL, prog);
++}
++
+ /* Match */
+ 
+ static int isnewline(int c)
+diff --git a/thirdparty/mujs/regexp.h b/thirdparty/mujs/regexp.h
+index 4bb4615..6bb73e8 100644
+--- a/thirdparty/mujs/regexp.h
++++ b/thirdparty/mujs/regexp.h
+@@ -1,6 +1,8 @@
+ #ifndef regexp_h
+ #define regexp_h
+ 
++#define regcompx js_regcompx
++#define regfreex js_regfreex
+ #define regcomp js_regcomp
+ #define regexec js_regexec
+ #define regfree js_regfree
+@@ -8,6 +10,11 @@
+ typedef struct Reprog Reprog;
+ typedef struct Resub Resub;
+ 
++Reprog *regcompx(void *(*alloc)(void *ctx, void *p, int n), void *ctx,
++	const char *pattern, int cflags, const char **errorp);
++void regfreex(void *(*alloc)(void *ctx, void *p, int n), void *ctx,
++	Reprog *prog);
++
+ Reprog *regcomp(const char *pattern, int cflags, const char **errorp);
+ int regexec(Reprog *prog, const char *string, Resub *sub, int eflags);
+ void regfree(Reprog *prog);
+-- 
+2.9.1
+
diff --git a/gnu/packages/pdf.scm b/gnu/packages/pdf.scm
index 9b3571e67..3388cbaa9 100644
--- a/gnu/packages/pdf.scm
+++ b/gnu/packages/pdf.scm
@@ -489,7 +489,9 @@ extracting content or merging files.")
         (sha256
          (base32
           "0dm8wcs8i29aibzkqkrn8kcnk4q0kd1v66pg48h5c3qqp4v1zk5a"))
-        (patches (search-patches "mupdf-build-with-openjpeg-2.1.patch"))
+        (patches (search-patches "mupdf-build-with-openjpeg-2.1.patch"
+                                 "mupdf-mujs-null-pointer-dereference.patch"
+                                 "mupdf-mujs-heap-buffer-overflow.patch"))
         (modules '((guix build utils)))
         (snippet
             ;; Delete all the bundled libraries except for mujs, which is
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH] gnu: mupdf: Fix some security problems in bundled mujs.
  2017-01-12 18:06 [PATCH] gnu: mupdf: Fix some security problems in bundled mujs Marius Bakke
@ 2017-01-12 18:30 ` Leo Famulari
  2017-01-12 19:46   ` Marius Bakke
  0 siblings, 1 reply; 13+ messages in thread
From: Leo Famulari @ 2017-01-12 18:30 UTC (permalink / raw)
  To: Marius Bakke; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 683 bytes --]

On Thu, Jan 12, 2017 at 07:06:55PM +0100, Marius Bakke wrote:
> * gnu/packages/patches/mupdf-mujs-heap-buffer-overflow.patch: New file.
> * gnu/packages/patches/mupdf-mujs-null-pointer-dereference.patch: New file.
> * gnu/local.mk (dist_patch_DATA): Add them.
> * gnu/packages/pdf.scm (mupdf)[source]: Use them.

Thanks!

I'd write it like this, but it's not important:

* gnu/packages/patches/mupdf-mujs-heap-buffer-overflow.patch,
gnu/packages/patches/mupdf-mujs-null-pointer-dereference.patch: New file.

Can you include links to the upstream bug reports in the patch files?

Through cups, this requires ~600 rebuilds. I wonder if we can graft it?
That is, is the ABI compatible?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] gnu: mupdf: Fix some security problems in bundled mujs.
  2017-01-12 18:30 ` Leo Famulari
@ 2017-01-12 19:46   ` Marius Bakke
  2017-01-12 20:03     ` Leo Famulari
  0 siblings, 1 reply; 13+ messages in thread
From: Marius Bakke @ 2017-01-12 19:46 UTC (permalink / raw)
  To: Leo Famulari; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 829 bytes --]

Leo Famulari <leo@famulari.name> writes:

> Can you include links to the upstream bug reports in the patch files?

Good catch; added.

> Through cups, this requires ~600 rebuilds. I wonder if we can graft it?
> That is, is the ABI compatible?

Good question. The null pointer dereference patch renames a function,
and I can find it in /gnu/store/...-mupdf-1.10a/lib/libmupdfthird.a. So
I guess not.

There is also /lib/libmupdf.a which I assume most packages use, and does
not seem to use anything from mujs.

This package only provides static libraries, so grafting may not even
work. In most cases I've come across, the static library is embedded
with "ar" in the final package (cups do not retain a rerefence to
mupdf). What to do?

(as an aside, I wonder if we can add an "ar-wrapper" that creates thin
archives by default).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] gnu: mupdf: Fix some security problems in bundled mujs.
  2017-01-12 19:46   ` Marius Bakke
@ 2017-01-12 20:03     ` Leo Famulari
  2017-01-13  0:59       ` Mark H Weaver
  0 siblings, 1 reply; 13+ messages in thread
From: Leo Famulari @ 2017-01-12 20:03 UTC (permalink / raw)
  To: Marius Bakke; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 952 bytes --]

On Thu, Jan 12, 2017 at 08:46:52PM +0100, Marius Bakke wrote:
> Leo Famulari <leo@famulari.name> writes:
> 
> > Can you include links to the upstream bug reports in the patch files?
> 
> Good catch; added.
> 
> > Through cups, this requires ~600 rebuilds. I wonder if we can graft it?
> > That is, is the ABI compatible?
> 
> Good question. The null pointer dereference patch renames a function,
> and I can find it in /gnu/store/...-mupdf-1.10a/lib/libmupdfthird.a. So
> I guess not.
> 
> There is also /lib/libmupdf.a which I assume most packages use, and does
> not seem to use anything from mujs.
> 
> This package only provides static libraries, so grafting may not even
> work. In most cases I've come across, the static library is embedded
> with "ar" in the final package (cups do not retain a rerefence to
> mupdf). What to do?

If we can't graft it, we should build it on a branch on Hydra.

Mark, what do you think?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] gnu: mupdf: Fix some security problems in bundled mujs.
  2017-01-12 20:03     ` Leo Famulari
@ 2017-01-13  0:59       ` Mark H Weaver
  2017-01-13 17:34         ` Leo Famulari
  2017-01-15 18:47         ` Leo Famulari
  0 siblings, 2 replies; 13+ messages in thread
From: Mark H Weaver @ 2017-01-13  0:59 UTC (permalink / raw)
  To: Leo Famulari; +Cc: guix-devel

Leo Famulari <leo@famulari.name> writes:

> On Thu, Jan 12, 2017 at 08:46:52PM +0100, Marius Bakke wrote:
>> Leo Famulari <leo@famulari.name> writes:
>> 
>> > Through cups, this requires ~600 rebuilds. I wonder if we can graft it?
>> > That is, is the ABI compatible?
>> 
>> Good question. The null pointer dereference patch renames a function,
>> and I can find it in /gnu/store/...-mupdf-1.10a/lib/libmupdfthird.a. So
>> I guess not.
>> 
>> There is also /lib/libmupdf.a which I assume most packages use, and does
>> not seem to use anything from mujs.
>> 
>> This package only provides static libraries, so grafting may not even
>> work. In most cases I've come across, the static library is embedded
>> with "ar" in the final package (cups do not retain a rerefence to
>> mupdf). What to do?
>
> If we can't graft it, we should build it on a branch on Hydra.
>
> Mark, what do you think?

Here's what we can do: in addition to mupdf itself, we can also add a
graft for cups-filters (our only package that includes mupdf as an
input).  The replacement for cups-filters would change its mupdf input
to refer directly to the fixed version of mupdf.

What do you think?

      Mark

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] gnu: mupdf: Fix some security problems in bundled mujs.
  2017-01-13  0:59       ` Mark H Weaver
@ 2017-01-13 17:34         ` Leo Famulari
  2017-01-15  8:20           ` Mark H Weaver
  2017-01-15 18:47         ` Leo Famulari
  1 sibling, 1 reply; 13+ messages in thread
From: Leo Famulari @ 2017-01-13 17:34 UTC (permalink / raw)
  To: Mark H Weaver; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 2760 bytes --]

On Thu, Jan 12, 2017 at 07:59:40PM -0500, Mark H Weaver wrote:
> Leo Famulari <leo@famulari.name> writes:
> > If we can't graft it, we should build it on a branch on Hydra.
> 
> Here's what we can do: in addition to mupdf itself, we can also add a
> graft for cups-filters (our only package that includes mupdf as an
> input).  The replacement for cups-filters would change its mupdf input
> to refer directly to the fixed version of mupdf.
> 
> What do you think?

That's a good idea, and I started implementing it, but then I wondered
how cups-filters was actually using mupdf. The cups-filters package is
only 3.7 MB, while libmupdf.a is 44 MB.

It turns out that the built cups-filters doesn't refer to mupdf at all;
mupdf is not protected from the garbage collector if you install
cups-filters.

I found two source files that use mupdf, 'filter/mupdftoraster.c' and
'filter/pdftops.c'.

From the cups-filters ./configure summary [0]:

------
mutool:          yes
mutool-path:     system
ippfind-path:    system
imagefilters:    yes
jpeg:            yes
pdftocairo-path: system
pdftops:         hybrid
------

The pdftops hybrid mode uses Poppler and Ghostscript, so we're not
affected here:

------
  if (renderer == HYBRID)
  {
    if (make_model[0] &&
        (!strncasecmp(make_model, "Brother", 7) ||
         !strncasecmp(make_model, "Dell", 4) ||
         strcasestr(make_model, "Minolta") ||
         (!strncasecmp(make_model, "Apple", 5) &&
          (ptr = strcasestr(make_model, "LaserWriter")) &&
          (ptr = strcasestr(ptr + 11, "12")) &&
          (ptr = strcasestr(ptr + 2, "640")))))
    {    
      fprintf(stderr, "DEBUG: Switching to Poppler's pdftops instead of Ghostscript for Brother, Minolta, Konica Minolta, Dell, and Apple LaserWriter 12/640 to work around bugs in the printer's PS interpreters\n");
      renderer = PDFTOPS;
    }    
    else 
      renderer = GS;
[...]
------
source:
<http://bzr.linuxfoundation.org/loggerhead/openprinting/cups-filters/annotate/head:/filter/pdftops.c#L467>

'filter/mupdfraster.c' involves the use of the mutool program. Does
the configuration option 'mutool-path: system' mean that it looks for
mutool on PATH?

config.log [1] has:
#define CUPS_MUTOOL "mutool"

And I can't find a store reference for mupdf with `hexdump -C
lib/cups/filter/mupdftoraster`; that's only file that `grep -ri mutool`
matches. 

Should we make cups-filters refer to mutool by an absolute path?

[0]
https://mirror.hydra.gnu.org/log/xlb7k5l3l4gq12z4fmg5i59y5hdzn472-cups-filters-1.13.1

[1]
config.log also has this line:

#define CUPS_POPPLER_PDFTOPS "/usr/bin/pdftops"'

This does get into the built 'lib/cups/filter/pdftops'.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] gnu: mupdf: Fix some security problems in bundled mujs.
  2017-01-13 17:34         ` Leo Famulari
@ 2017-01-15  8:20           ` Mark H Weaver
  0 siblings, 0 replies; 13+ messages in thread
From: Mark H Weaver @ 2017-01-15  8:20 UTC (permalink / raw)
  To: Leo Famulari; +Cc: guix-devel

Leo Famulari <leo@famulari.name> writes:

> On Thu, Jan 12, 2017 at 07:59:40PM -0500, Mark H Weaver wrote:
>> Leo Famulari <leo@famulari.name> writes:
>> > If we can't graft it, we should build it on a branch on Hydra.
>> 
>> Here's what we can do: in addition to mupdf itself, we can also add a
>> graft for cups-filters (our only package that includes mupdf as an
>> input).  The replacement for cups-filters would change its mupdf input
>> to refer directly to the fixed version of mupdf.
>> 
>> What do you think?
>
> That's a good idea, and I started implementing it, but then I wondered
> how cups-filters was actually using mupdf. The cups-filters package is
> only 3.7 MB, while libmupdf.a is 44 MB.
>
> It turns out that the built cups-filters doesn't refer to mupdf at all;
> mupdf is not protected from the garbage collector if you install
> cups-filters.

Static linking copies segments of code and data from the *.a into
whatever is being linked (an executable or library).  So, buggy code
might be copied from libmupdf.a into 'cups-filters', with no references
to 'mupdf' remaining.  Also, the fact that cups-filters is smaller than
libmupdf.a doesn't prove that code wasn't copied from libmupdf.a.

      Thanks,
        Mark

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] gnu: mupdf: Fix some security problems in bundled mujs.
  2017-01-13  0:59       ` Mark H Weaver
  2017-01-13 17:34         ` Leo Famulari
@ 2017-01-15 18:47         ` Leo Famulari
  2017-01-15 19:05           ` Marius Bakke
  1 sibling, 1 reply; 13+ messages in thread
From: Leo Famulari @ 2017-01-15 18:47 UTC (permalink / raw)
  To: Mark H Weaver; +Cc: guix-devel


[-- Attachment #1.1: Type: text/plain, Size: 409 bytes --]

On Thu, Jan 12, 2017 at 07:59:40PM -0500, Mark H Weaver wrote:
> Here's what we can do: in addition to mupdf itself, we can also add a
> graft for cups-filters (our only package that includes mupdf as an
> input).  The replacement for cups-filters would change its mupdf input
> to refer directly to the fixed version of mupdf.
> 
> What do you think?

I've attached two patches that should do this.

[-- Attachment #1.2: 0001-gnu-mupdf-Fix-CVE-2016-10132-10133-in-bundled-mujs.patch --]
[-- Type: text/plain, Size: 10498 bytes --]

From 4216ccff0b032bdad8c730ba9929b94f389fb19d Mon Sep 17 00:00:00 2001
From: Marius Bakke <mbakke@fastmail.com>
Date: Thu, 12 Jan 2017 19:06:55 +0100
Subject: [PATCH 1/2] gnu: mupdf: Fix CVE-2016-{10132,10133} in bundled mujs.

* gnu/packages/patches/mupdf-mujs-CVE-2016-10132.patch,
gnu/packages/patches/mupdf-mujs-CVE-2016-10133.patch: New files.
* gnu/local.mk (dist_patch_DATA): Add them.
* gnu/packages/pdf.scm (mupdf)[replacement]: New field.
(mupdf/fixed): New variable.
---
 gnu/local.mk                                       |   2 +
 .../patches/mupdf-mujs-CVE-2016-10132.patch        | 188 +++++++++++++++++++++
 .../patches/mupdf-mujs-CVE-2016-10133.patch        |  36 ++++
 gnu/packages/pdf.scm                               |  15 +-
 4 files changed, 240 insertions(+), 1 deletion(-)
 create mode 100644 gnu/packages/patches/mupdf-mujs-CVE-2016-10132.patch
 create mode 100644 gnu/packages/patches/mupdf-mujs-CVE-2016-10133.patch

diff --git a/gnu/local.mk b/gnu/local.mk
index 81d774eb6..58554160d 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -755,6 +755,8 @@ dist_patch_DATA =						\
   %D%/packages/patches/multiqc-fix-git-subprocess-error.patch	\
   %D%/packages/patches/mumps-build-parallelism.patch		\
   %D%/packages/patches/mupdf-build-with-openjpeg-2.1.patch	\
+  %D%/packages/patches/mupdf-mujs-CVE-2016-10132.patch		\
+  %D%/packages/patches/mupdf-mujs-CVE-2016-10133.patch		\
   %D%/packages/patches/mupen64plus-ui-console-notice.patch	\
   %D%/packages/patches/musl-CVE-2016-8859.patch			\
   %D%/packages/patches/mutt-store-references.patch		\
diff --git a/gnu/packages/patches/mupdf-mujs-CVE-2016-10132.patch b/gnu/packages/patches/mupdf-mujs-CVE-2016-10132.patch
new file mode 100644
index 000000000..e752e57ec
--- /dev/null
+++ b/gnu/packages/patches/mupdf-mujs-CVE-2016-10132.patch
@@ -0,0 +1,188 @@
+Fix CVE-2016-10132:
+
+https://bugs.ghostscript.com/show_bug.cgi?id=697381
+http://seclists.org/oss-sec/2017/q1/74
+https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2016-10132
+
+Patch lifted from upstream source repository:
+
+http://git.ghostscript.com/?p=mujs.git;h=fd003eceda531e13fbdd1aeb6e9c73156496e569
+
+From fd003eceda531e13fbdd1aeb6e9c73156496e569 Mon Sep 17 00:00:00 2001
+From: Tor Andersson <tor@ccxvii.net>
+Date: Fri, 2 Dec 2016 14:56:20 -0500
+Subject: [PATCH] Fix 697381: check allocation when compiling regular
+ expressions.
+
+Also use allocator callback function.
+---
+ thirdparty/mujs/jsgc.c     |  2 +-
+ thirdparty/mujs/jsregexp.c |  2 +-
+ thirdparty/mujs/jsstate.c  |  6 ------
+ thirdparty/mujs/regexp.c   | 45 +++++++++++++++++++++++++++++++++++----------
+ thirdparty/mujs/regexp.h   |  7 +++++++
+ 5 files changed, 44 insertions(+), 18 deletions(-)
+
+diff --git a/thirdparty/mujs/jsgc.c b/thirdparty/mujs/jsgc.c
+index 4f7e7dc..f80111e 100644
+--- a/thirdparty/mujs/jsgc.c
++++ b/thirdparty/mujs/jsgc.c
+@@ -46,7 +46,7 @@ static void jsG_freeobject(js_State *J, js_Object *obj)
+ 		jsG_freeproperty(J, obj->head);
+ 	if (obj->type == JS_CREGEXP) {
+ 		js_free(J, obj->u.r.source);
+-		js_regfree(obj->u.r.prog);
++		js_regfreex(J->alloc, J->actx, obj->u.r.prog);
+ 	}
+ 	if (obj->type == JS_CITERATOR)
+ 		jsG_freeiterator(J, obj->u.iter.head);
+diff --git a/thirdparty/mujs/jsregexp.c b/thirdparty/mujs/jsregexp.c
+index a2d5156..7b09c06 100644
+--- a/thirdparty/mujs/jsregexp.c
++++ b/thirdparty/mujs/jsregexp.c
+@@ -16,7 +16,7 @@ void js_newregexp(js_State *J, const char *pattern, int flags)
+ 	if (flags & JS_REGEXP_I) opts |= REG_ICASE;
+ 	if (flags & JS_REGEXP_M) opts |= REG_NEWLINE;
+ 
+-	prog = js_regcomp(pattern, opts, &error);
++	prog = js_regcompx(J->alloc, J->actx, pattern, opts, &error);
+ 	if (!prog)
+ 		js_syntaxerror(J, "regular expression: %s", error);
+ 
+diff --git a/thirdparty/mujs/jsstate.c b/thirdparty/mujs/jsstate.c
+index 638cab3..fd5bcf6 100644
+--- a/thirdparty/mujs/jsstate.c
++++ b/thirdparty/mujs/jsstate.c
+@@ -9,12 +9,6 @@
+ 
+ static void *js_defaultalloc(void *actx, void *ptr, int size)
+ {
+-	if (size == 0) {
+-		free(ptr);
+-		return NULL;
+-	}
+-	if (!ptr)
+-		return malloc((size_t)size);
+ 	return realloc(ptr, (size_t)size);
+ }
+ 
+diff --git a/thirdparty/mujs/regexp.c b/thirdparty/mujs/regexp.c
+index 9852be2..01c18a3 100644
+--- a/thirdparty/mujs/regexp.c
++++ b/thirdparty/mujs/regexp.c
+@@ -807,23 +807,31 @@ static void dumpprog(Reprog *prog)
+ }
+ #endif
+ 
+-Reprog *regcomp(const char *pattern, int cflags, const char **errorp)
++Reprog *regcompx(void *(*alloc)(void *ctx, void *p, int n), void *ctx,
++	const char *pattern, int cflags, const char **errorp)
+ {
+ 	struct cstate g;
+ 	Renode *node;
+ 	Reinst *split, *jump;
+ 	int i;
+ 
+-	g.prog = malloc(sizeof (Reprog));
+-	g.pstart = g.pend = malloc(sizeof (Renode) * strlen(pattern) * 2);
++	g.pstart = NULL;
++	g.prog = NULL;
+ 
+ 	if (setjmp(g.kaboom)) {
+ 		if (errorp) *errorp = g.error;
+-		free(g.pstart);
+-		free(g.prog);
++		alloc(ctx, g.pstart, 0);
++		alloc(ctx, g.prog, 0);
+ 		return NULL;
+ 	}
+ 
++	g.prog = alloc(ctx, NULL, sizeof (Reprog));
++	if (!g.prog)
++		die(&g, "cannot allocate regular expression");
++	g.pstart = g.pend = alloc(ctx, NULL, sizeof (Renode) * strlen(pattern) * 2);
++	if (!g.pstart)
++		die(&g, "cannot allocate regular expression parse list");
++
+ 	g.source = pattern;
+ 	g.ncclass = 0;
+ 	g.nsub = 1;
+@@ -840,7 +848,9 @@ Reprog *regcomp(const char *pattern, int cflags, const char **errorp)
+ 		die(&g, "syntax error");
+ 
+ 	g.prog->nsub = g.nsub;
+-	g.prog->start = g.prog->end = malloc((count(node) + 6) * sizeof (Reinst));
++	g.prog->start = g.prog->end = alloc(ctx, NULL, (count(node) + 6) * sizeof (Reinst));
++	if (!g.prog->start)
++		die(&g, "cannot allocate regular expression instruction list");
+ 
+ 	split = emit(g.prog, I_SPLIT);
+ 	split->x = split + 3;
+@@ -859,20 +869,35 @@ Reprog *regcomp(const char *pattern, int cflags, const char **errorp)
+ 	dumpprog(g.prog);
+ #endif
+ 
+-	free(g.pstart);
++	alloc(ctx, g.pstart, 0);
+ 
+ 	if (errorp) *errorp = NULL;
+ 	return g.prog;
+ }
+ 
+-void regfree(Reprog *prog)
++void regfreex(void *(*alloc)(void *ctx, void *p, int n), void *ctx, Reprog *prog)
+ {
+ 	if (prog) {
+-		free(prog->start);
+-		free(prog);
++		alloc(ctx, prog->start, 0);
++		alloc(ctx, prog, 0);
+ 	}
+ }
+ 
++static void *default_alloc(void *ctx, void *p, int n)
++{
++	return realloc(p, (size_t)n);
++}
++
++Reprog *regcomp(const char *pattern, int cflags, const char **errorp)
++{
++	return regcompx(default_alloc, NULL, pattern, cflags, errorp);
++}
++
++void regfree(Reprog *prog)
++{
++	regfreex(default_alloc, NULL, prog);
++}
++
+ /* Match */
+ 
+ static int isnewline(int c)
+diff --git a/thirdparty/mujs/regexp.h b/thirdparty/mujs/regexp.h
+index 4bb4615..6bb73e8 100644
+--- a/thirdparty/mujs/regexp.h
++++ b/thirdparty/mujs/regexp.h
+@@ -1,6 +1,8 @@
+ #ifndef regexp_h
+ #define regexp_h
+ 
++#define regcompx js_regcompx
++#define regfreex js_regfreex
+ #define regcomp js_regcomp
+ #define regexec js_regexec
+ #define regfree js_regfree
+@@ -8,6 +10,11 @@
+ typedef struct Reprog Reprog;
+ typedef struct Resub Resub;
+ 
++Reprog *regcompx(void *(*alloc)(void *ctx, void *p, int n), void *ctx,
++	const char *pattern, int cflags, const char **errorp);
++void regfreex(void *(*alloc)(void *ctx, void *p, int n), void *ctx,
++	Reprog *prog);
++
+ Reprog *regcomp(const char *pattern, int cflags, const char **errorp);
+ int regexec(Reprog *prog, const char *string, Resub *sub, int eflags);
+ void regfree(Reprog *prog);
+-- 
+2.9.1
+
diff --git a/gnu/packages/patches/mupdf-mujs-CVE-2016-10133.patch b/gnu/packages/patches/mupdf-mujs-CVE-2016-10133.patch
new file mode 100644
index 000000000..d73849262
--- /dev/null
+++ b/gnu/packages/patches/mupdf-mujs-CVE-2016-10133.patch
@@ -0,0 +1,36 @@
+Fix CVE-2016-10133:
+
+https://bugs.ghostscript.com/show_bug.cgi?id=697401
+http://seclists.org/oss-sec/2017/q1/74
+https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2016-10133
+
+Patch lifted from upstream source repository:
+
+https://git.ghostscript.com/?p=mujs.git;h=77ab465f1c394bb77f00966cd950650f3f53cb24
+
+From 77ab465f1c394bb77f00966cd950650f3f53cb24 Mon Sep 17 00:00:00 2001
+From: Tor Andersson <tor.andersson@gmail.com>
+Date: Thu, 12 Jan 2017 14:47:01 +0100
+Subject: [PATCH] Fix 697401: Error when dropping extra arguments to
+ lightweight functions.
+
+---
+ thirdparty/mujs/jsrun.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/thirdparty/mujs/jsrun.c b/thirdparty/mujs/jsrun.c
+index ee80845..782a6f9 100644
+--- a/thirdparty/mujs/jsrun.c
++++ b/thirdparty/mujs/jsrun.c
+@@ -937,7 +937,7 @@ static void jsR_calllwfunction(js_State *J, int n, js_Function *F, js_Environmen
+ 	jsR_savescope(J, scope);
+ 
+ 	if (n > F->numparams) {
+-		js_pop(J, F->numparams - n);
++		js_pop(J, n - F->numparams);
+ 		n = F->numparams;
+ 	}
+ 	for (i = n; i < F->varlen; ++i)
+-- 
+2.9.1
+
diff --git a/gnu/packages/pdf.scm b/gnu/packages/pdf.scm
index 9b3571e67..5efc5e6d1 100644
--- a/gnu/packages/pdf.scm
+++ b/gnu/packages/pdf.scm
@@ -6,10 +6,11 @@
 ;;; Copyright © 2016 Roel Janssen <roel@gnu.org>
 ;;; Coypright © 2016 ng0 <ng0@we.make.ritual.n0.is>
 ;;; Coypright © 2016 Efraim Flashner <efraim@flashner.co.il>
-;;; Coypright © 2016 Marius Bakke <mbakke@fastmail.com>
+;;; Coypright © 2016, 2017 Marius Bakke <mbakke@fastmail.com>
 ;;; Coypright © 2016 Ludovic Courtès <ludo@gnu.org>
 ;;; Coypright © 2016 Julien Lepiller <julien@lepiller.eu>
 ;;; Copyright © 2016 Arun Isaac <arunisaac@systemreboot.net>
+;;; Copyright © 2017 Leo Famulari <leo@famulari.name>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -538,6 +539,18 @@ line tools for batch rendering (pdfdraw), rewriting files (pdfclean),
 and examining the file structure (pdfshow).")
     (license license:agpl3+)))
 
+(define mupdf/fixed
+  (package
+    (inherit mupdf)
+    (source
+      (origin
+        (inherit (package-source mupdf))
+        (patches
+          (append
+            (origin-patches (package-source mupdf))
+            (search-patches "mupdf-mujs-CVE-2016-10132.patch"
+                            "mupdf-mujs-CVE-2016-10133.patch")))))))
+
 (define-public qpdf
   (package
    (name "qpdf")
-- 
2.11.0


[-- Attachment #1.3: 0002-gnu-cups-filters-Fix-CVE-2016-10132-10133-in-statica.patch --]
[-- Type: text/plain, Size: 2291 bytes --]

From a656359de1e7d0a76414888a59c8a0a8782e875f Mon Sep 17 00:00:00 2001
From: Leo Famulari <leo@famulari.name>
Date: Sun, 15 Jan 2017 13:38:48 -0500
Subject: [PATCH 2/2] gnu: cups-filters: Fix CVE-2016-{10132,10133} in
 statically linked mupdf.

The vulnerabilities are the MuJS that is bundled with MuPDF.

* gnu/packages/cups.scm (cups-filters)[replacement]: New field.
(cups-filters/fixed): New variable.
---
 gnu/packages/cups.scm | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/gnu/packages/cups.scm b/gnu/packages/cups.scm
index ca1695835..95d57a4f3 100644
--- a/gnu/packages/cups.scm
+++ b/gnu/packages/cups.scm
@@ -3,6 +3,7 @@
 ;;; Copyright © 2015, 2016 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2015, 2016 Efraim Flashner <efraim@flashner.co.il>
 ;;; Copyright © 2016 Danny Milosavljevic <dannym@scratchpost.org>
+;;; Copyright © 2017 Leo Famulari <leo@famulari.name>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -51,6 +52,7 @@
 (define-public cups-filters
   (package
     (name "cups-filters")
+    (replacement cups-filters/fixed)
     (version "1.13.1")
     (source(origin
               (method url-fetch)
@@ -133,6 +135,27 @@ filters for the PDF-centric printing workflow introduced by OpenPrinting.")
                    license:lgpl2.0+
                    license:expat))))
 
+(define cups-filters/fixed
+  (package
+    (inherit cups-filters)
+    (inputs
+     `(("avahi"        ,avahi)
+       ("fontconfig"   ,fontconfig)
+       ("freetype"     ,freetype)
+       ("font-dejavu"  ,font-dejavu) ; also needed by test suite
+       ("ghostscript"  ,(force ghostscript/cups))
+       ("ijs"          ,ijs)
+       ("dbus"         ,dbus)
+       ("lcms"         ,lcms)
+       ("libjpeg"      ,libjpeg)
+       ("libpng"       ,libpng)
+       ("libtiff"      ,libtiff)
+       ("mupdf"        ,(@@ (gnu packages pdf) mupdf/fixed))
+       ("glib"         ,glib)
+       ("qpdf"         ,qpdf)
+       ("poppler"      ,poppler)
+       ("cups-minimal" ,cups-minimal)))))
+
 ;; CUPS on non-MacOS systems requires cups-filters.  Since cups-filters also
 ;; depends on CUPS libraries and binaries, cups-minimal has been added to
 ;; satisfy this dependency.
-- 
2.11.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH] gnu: mupdf: Fix some security problems in bundled mujs.
  2017-01-15 18:47         ` Leo Famulari
@ 2017-01-15 19:05           ` Marius Bakke
  2017-01-15 20:49             ` Leo Famulari
  0 siblings, 1 reply; 13+ messages in thread
From: Marius Bakke @ 2017-01-15 19:05 UTC (permalink / raw)
  To: Leo Famulari, Mark H Weaver; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 1473 bytes --]

Leo Famulari <leo@famulari.name> writes:

> On Thu, Jan 12, 2017 at 07:59:40PM -0500, Mark H Weaver wrote:
>> Here's what we can do: in addition to mupdf itself, we can also add a
>> graft for cups-filters (our only package that includes mupdf as an
>> input).  The replacement for cups-filters would change its mupdf input
>> to refer directly to the fixed version of mupdf.
>> 
>> What do you think?
>
> I've attached two patches that should do this.

Thanks for doing this!
  
> +(define cups-filters/fixed
> +  (package
> +    (inherit cups-filters)
> +    (inputs
> +     `(("avahi"        ,avahi)
> +       ("fontconfig"   ,fontconfig)
> +       ("freetype"     ,freetype)
> +       ("font-dejavu"  ,font-dejavu) ; also needed by test suite
> +       ("ghostscript"  ,(force ghostscript/cups))
> +       ("ijs"          ,ijs)
> +       ("dbus"         ,dbus)
> +       ("lcms"         ,lcms)
> +       ("libjpeg"      ,libjpeg)
> +       ("libpng"       ,libpng)
> +       ("libtiff"      ,libtiff)
> +       ("mupdf"        ,(@@ (gnu packages pdf) mupdf/fixed))
> +       ("glib"         ,glib)
> +       ("qpdf"         ,qpdf)
> +       ("poppler"      ,poppler)
> +       ("cups-minimal" ,cups-minimal)))))

Is it possible to use the 'package-input-rewriting' procedure here? See
example at the end of section 5.1.0:

https://www.gnu.org/software/guix/manual/guix.html#Defining-Packages

Otherwise this LGTM, thanks a lot!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] gnu: mupdf: Fix some security problems in bundled mujs.
  2017-01-15 19:05           ` Marius Bakke
@ 2017-01-15 20:49             ` Leo Famulari
  2017-01-15 20:56               ` Marius Bakke
  2017-01-15 23:05               ` Mark H Weaver
  0 siblings, 2 replies; 13+ messages in thread
From: Leo Famulari @ 2017-01-15 20:49 UTC (permalink / raw)
  To: Marius Bakke; +Cc: guix-devel


[-- Attachment #1.1: Type: text/plain, Size: 346 bytes --]

On Sun, Jan 15, 2017 at 08:05:48PM +0100, Marius Bakke wrote:
> Is it possible to use the 'package-input-rewriting' procedure here? See
> example at the end of section 5.1.0:
> 
> https://www.gnu.org/software/guix/manual/guix.html#Defining-Packages
> 
> Otherwise this LGTM, thanks a lot!

Okay, please test this updated patch series :)

[-- Attachment #1.2: 0001-gnu-mupdf-Fix-CVE-2016-10132-10133-in-bundled-mujs.patch --]
[-- Type: text/plain, Size: 10498 bytes --]

From 34cc0dc9d9451d540f8733ebca2a3db54a073aa0 Mon Sep 17 00:00:00 2001
From: Marius Bakke <mbakke@fastmail.com>
Date: Thu, 12 Jan 2017 19:06:55 +0100
Subject: [PATCH 1/2] gnu: mupdf: Fix CVE-2016-{10132,10133} in bundled mujs.

* gnu/packages/patches/mupdf-mujs-CVE-2016-10132.patch,
gnu/packages/patches/mupdf-mujs-CVE-2016-10133.patch: New files.
* gnu/local.mk (dist_patch_DATA): Add them.
* gnu/packages/pdf.scm (mupdf)[replacement]: New field.
(mupdf/fixed): New variable.
---
 gnu/local.mk                                       |   2 +
 .../patches/mupdf-mujs-CVE-2016-10132.patch        | 188 +++++++++++++++++++++
 .../patches/mupdf-mujs-CVE-2016-10133.patch        |  36 ++++
 gnu/packages/pdf.scm                               |  15 +-
 4 files changed, 240 insertions(+), 1 deletion(-)
 create mode 100644 gnu/packages/patches/mupdf-mujs-CVE-2016-10132.patch
 create mode 100644 gnu/packages/patches/mupdf-mujs-CVE-2016-10133.patch

diff --git a/gnu/local.mk b/gnu/local.mk
index 81d774eb6..58554160d 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -755,6 +755,8 @@ dist_patch_DATA =						\
   %D%/packages/patches/multiqc-fix-git-subprocess-error.patch	\
   %D%/packages/patches/mumps-build-parallelism.patch		\
   %D%/packages/patches/mupdf-build-with-openjpeg-2.1.patch	\
+  %D%/packages/patches/mupdf-mujs-CVE-2016-10132.patch		\
+  %D%/packages/patches/mupdf-mujs-CVE-2016-10133.patch		\
   %D%/packages/patches/mupen64plus-ui-console-notice.patch	\
   %D%/packages/patches/musl-CVE-2016-8859.patch			\
   %D%/packages/patches/mutt-store-references.patch		\
diff --git a/gnu/packages/patches/mupdf-mujs-CVE-2016-10132.patch b/gnu/packages/patches/mupdf-mujs-CVE-2016-10132.patch
new file mode 100644
index 000000000..e752e57ec
--- /dev/null
+++ b/gnu/packages/patches/mupdf-mujs-CVE-2016-10132.patch
@@ -0,0 +1,188 @@
+Fix CVE-2016-10132:
+
+https://bugs.ghostscript.com/show_bug.cgi?id=697381
+http://seclists.org/oss-sec/2017/q1/74
+https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2016-10132
+
+Patch lifted from upstream source repository:
+
+http://git.ghostscript.com/?p=mujs.git;h=fd003eceda531e13fbdd1aeb6e9c73156496e569
+
+From fd003eceda531e13fbdd1aeb6e9c73156496e569 Mon Sep 17 00:00:00 2001
+From: Tor Andersson <tor@ccxvii.net>
+Date: Fri, 2 Dec 2016 14:56:20 -0500
+Subject: [PATCH] Fix 697381: check allocation when compiling regular
+ expressions.
+
+Also use allocator callback function.
+---
+ thirdparty/mujs/jsgc.c     |  2 +-
+ thirdparty/mujs/jsregexp.c |  2 +-
+ thirdparty/mujs/jsstate.c  |  6 ------
+ thirdparty/mujs/regexp.c   | 45 +++++++++++++++++++++++++++++++++++----------
+ thirdparty/mujs/regexp.h   |  7 +++++++
+ 5 files changed, 44 insertions(+), 18 deletions(-)
+
+diff --git a/thirdparty/mujs/jsgc.c b/thirdparty/mujs/jsgc.c
+index 4f7e7dc..f80111e 100644
+--- a/thirdparty/mujs/jsgc.c
++++ b/thirdparty/mujs/jsgc.c
+@@ -46,7 +46,7 @@ static void jsG_freeobject(js_State *J, js_Object *obj)
+ 		jsG_freeproperty(J, obj->head);
+ 	if (obj->type == JS_CREGEXP) {
+ 		js_free(J, obj->u.r.source);
+-		js_regfree(obj->u.r.prog);
++		js_regfreex(J->alloc, J->actx, obj->u.r.prog);
+ 	}
+ 	if (obj->type == JS_CITERATOR)
+ 		jsG_freeiterator(J, obj->u.iter.head);
+diff --git a/thirdparty/mujs/jsregexp.c b/thirdparty/mujs/jsregexp.c
+index a2d5156..7b09c06 100644
+--- a/thirdparty/mujs/jsregexp.c
++++ b/thirdparty/mujs/jsregexp.c
+@@ -16,7 +16,7 @@ void js_newregexp(js_State *J, const char *pattern, int flags)
+ 	if (flags & JS_REGEXP_I) opts |= REG_ICASE;
+ 	if (flags & JS_REGEXP_M) opts |= REG_NEWLINE;
+ 
+-	prog = js_regcomp(pattern, opts, &error);
++	prog = js_regcompx(J->alloc, J->actx, pattern, opts, &error);
+ 	if (!prog)
+ 		js_syntaxerror(J, "regular expression: %s", error);
+ 
+diff --git a/thirdparty/mujs/jsstate.c b/thirdparty/mujs/jsstate.c
+index 638cab3..fd5bcf6 100644
+--- a/thirdparty/mujs/jsstate.c
++++ b/thirdparty/mujs/jsstate.c
+@@ -9,12 +9,6 @@
+ 
+ static void *js_defaultalloc(void *actx, void *ptr, int size)
+ {
+-	if (size == 0) {
+-		free(ptr);
+-		return NULL;
+-	}
+-	if (!ptr)
+-		return malloc((size_t)size);
+ 	return realloc(ptr, (size_t)size);
+ }
+ 
+diff --git a/thirdparty/mujs/regexp.c b/thirdparty/mujs/regexp.c
+index 9852be2..01c18a3 100644
+--- a/thirdparty/mujs/regexp.c
++++ b/thirdparty/mujs/regexp.c
+@@ -807,23 +807,31 @@ static void dumpprog(Reprog *prog)
+ }
+ #endif
+ 
+-Reprog *regcomp(const char *pattern, int cflags, const char **errorp)
++Reprog *regcompx(void *(*alloc)(void *ctx, void *p, int n), void *ctx,
++	const char *pattern, int cflags, const char **errorp)
+ {
+ 	struct cstate g;
+ 	Renode *node;
+ 	Reinst *split, *jump;
+ 	int i;
+ 
+-	g.prog = malloc(sizeof (Reprog));
+-	g.pstart = g.pend = malloc(sizeof (Renode) * strlen(pattern) * 2);
++	g.pstart = NULL;
++	g.prog = NULL;
+ 
+ 	if (setjmp(g.kaboom)) {
+ 		if (errorp) *errorp = g.error;
+-		free(g.pstart);
+-		free(g.prog);
++		alloc(ctx, g.pstart, 0);
++		alloc(ctx, g.prog, 0);
+ 		return NULL;
+ 	}
+ 
++	g.prog = alloc(ctx, NULL, sizeof (Reprog));
++	if (!g.prog)
++		die(&g, "cannot allocate regular expression");
++	g.pstart = g.pend = alloc(ctx, NULL, sizeof (Renode) * strlen(pattern) * 2);
++	if (!g.pstart)
++		die(&g, "cannot allocate regular expression parse list");
++
+ 	g.source = pattern;
+ 	g.ncclass = 0;
+ 	g.nsub = 1;
+@@ -840,7 +848,9 @@ Reprog *regcomp(const char *pattern, int cflags, const char **errorp)
+ 		die(&g, "syntax error");
+ 
+ 	g.prog->nsub = g.nsub;
+-	g.prog->start = g.prog->end = malloc((count(node) + 6) * sizeof (Reinst));
++	g.prog->start = g.prog->end = alloc(ctx, NULL, (count(node) + 6) * sizeof (Reinst));
++	if (!g.prog->start)
++		die(&g, "cannot allocate regular expression instruction list");
+ 
+ 	split = emit(g.prog, I_SPLIT);
+ 	split->x = split + 3;
+@@ -859,20 +869,35 @@ Reprog *regcomp(const char *pattern, int cflags, const char **errorp)
+ 	dumpprog(g.prog);
+ #endif
+ 
+-	free(g.pstart);
++	alloc(ctx, g.pstart, 0);
+ 
+ 	if (errorp) *errorp = NULL;
+ 	return g.prog;
+ }
+ 
+-void regfree(Reprog *prog)
++void regfreex(void *(*alloc)(void *ctx, void *p, int n), void *ctx, Reprog *prog)
+ {
+ 	if (prog) {
+-		free(prog->start);
+-		free(prog);
++		alloc(ctx, prog->start, 0);
++		alloc(ctx, prog, 0);
+ 	}
+ }
+ 
++static void *default_alloc(void *ctx, void *p, int n)
++{
++	return realloc(p, (size_t)n);
++}
++
++Reprog *regcomp(const char *pattern, int cflags, const char **errorp)
++{
++	return regcompx(default_alloc, NULL, pattern, cflags, errorp);
++}
++
++void regfree(Reprog *prog)
++{
++	regfreex(default_alloc, NULL, prog);
++}
++
+ /* Match */
+ 
+ static int isnewline(int c)
+diff --git a/thirdparty/mujs/regexp.h b/thirdparty/mujs/regexp.h
+index 4bb4615..6bb73e8 100644
+--- a/thirdparty/mujs/regexp.h
++++ b/thirdparty/mujs/regexp.h
+@@ -1,6 +1,8 @@
+ #ifndef regexp_h
+ #define regexp_h
+ 
++#define regcompx js_regcompx
++#define regfreex js_regfreex
+ #define regcomp js_regcomp
+ #define regexec js_regexec
+ #define regfree js_regfree
+@@ -8,6 +10,11 @@
+ typedef struct Reprog Reprog;
+ typedef struct Resub Resub;
+ 
++Reprog *regcompx(void *(*alloc)(void *ctx, void *p, int n), void *ctx,
++	const char *pattern, int cflags, const char **errorp);
++void regfreex(void *(*alloc)(void *ctx, void *p, int n), void *ctx,
++	Reprog *prog);
++
+ Reprog *regcomp(const char *pattern, int cflags, const char **errorp);
+ int regexec(Reprog *prog, const char *string, Resub *sub, int eflags);
+ void regfree(Reprog *prog);
+-- 
+2.9.1
+
diff --git a/gnu/packages/patches/mupdf-mujs-CVE-2016-10133.patch b/gnu/packages/patches/mupdf-mujs-CVE-2016-10133.patch
new file mode 100644
index 000000000..d73849262
--- /dev/null
+++ b/gnu/packages/patches/mupdf-mujs-CVE-2016-10133.patch
@@ -0,0 +1,36 @@
+Fix CVE-2016-10133:
+
+https://bugs.ghostscript.com/show_bug.cgi?id=697401
+http://seclists.org/oss-sec/2017/q1/74
+https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2016-10133
+
+Patch lifted from upstream source repository:
+
+https://git.ghostscript.com/?p=mujs.git;h=77ab465f1c394bb77f00966cd950650f3f53cb24
+
+From 77ab465f1c394bb77f00966cd950650f3f53cb24 Mon Sep 17 00:00:00 2001
+From: Tor Andersson <tor.andersson@gmail.com>
+Date: Thu, 12 Jan 2017 14:47:01 +0100
+Subject: [PATCH] Fix 697401: Error when dropping extra arguments to
+ lightweight functions.
+
+---
+ thirdparty/mujs/jsrun.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/thirdparty/mujs/jsrun.c b/thirdparty/mujs/jsrun.c
+index ee80845..782a6f9 100644
+--- a/thirdparty/mujs/jsrun.c
++++ b/thirdparty/mujs/jsrun.c
+@@ -937,7 +937,7 @@ static void jsR_calllwfunction(js_State *J, int n, js_Function *F, js_Environmen
+ 	jsR_savescope(J, scope);
+ 
+ 	if (n > F->numparams) {
+-		js_pop(J, F->numparams - n);
++		js_pop(J, n - F->numparams);
+ 		n = F->numparams;
+ 	}
+ 	for (i = n; i < F->varlen; ++i)
+-- 
+2.9.1
+
diff --git a/gnu/packages/pdf.scm b/gnu/packages/pdf.scm
index 9b3571e67..5efc5e6d1 100644
--- a/gnu/packages/pdf.scm
+++ b/gnu/packages/pdf.scm
@@ -6,10 +6,11 @@
 ;;; Copyright © 2016 Roel Janssen <roel@gnu.org>
 ;;; Coypright © 2016 ng0 <ng0@we.make.ritual.n0.is>
 ;;; Coypright © 2016 Efraim Flashner <efraim@flashner.co.il>
-;;; Coypright © 2016 Marius Bakke <mbakke@fastmail.com>
+;;; Coypright © 2016, 2017 Marius Bakke <mbakke@fastmail.com>
 ;;; Coypright © 2016 Ludovic Courtès <ludo@gnu.org>
 ;;; Coypright © 2016 Julien Lepiller <julien@lepiller.eu>
 ;;; Copyright © 2016 Arun Isaac <arunisaac@systemreboot.net>
+;;; Copyright © 2017 Leo Famulari <leo@famulari.name>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -538,6 +539,18 @@ line tools for batch rendering (pdfdraw), rewriting files (pdfclean),
 and examining the file structure (pdfshow).")
     (license license:agpl3+)))
 
+(define mupdf/fixed
+  (package
+    (inherit mupdf)
+    (source
+      (origin
+        (inherit (package-source mupdf))
+        (patches
+          (append
+            (origin-patches (package-source mupdf))
+            (search-patches "mupdf-mujs-CVE-2016-10132.patch"
+                            "mupdf-mujs-CVE-2016-10133.patch")))))))
+
 (define-public qpdf
   (package
    (name "qpdf")
-- 
2.11.0


[-- Attachment #1.3: 0002-gnu-cups-filters-Fix-CVE-2016-10132-10133-in-statica.patch --]
[-- Type: text/plain, Size: 1793 bytes --]

From 58de4daf7fea3a2196fe055404d685930b2b57b4 Mon Sep 17 00:00:00 2001
From: Leo Famulari <leo@famulari.name>
Date: Sun, 15 Jan 2017 13:38:48 -0500
Subject: [PATCH 2/2] gnu: cups-filters: Fix CVE-2016-{10132,10133} in
 statically linked mupdf.

The vulnerabilities are the MuJS that is bundled with MuPDF.

* gnu/packages/cups.scm (cups-filters)[replacement]: New field.
(mupdf/fixed-instead-of-mupdf), (cups-filters/fixed): New variables.
---
 gnu/packages/cups.scm | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/gnu/packages/cups.scm b/gnu/packages/cups.scm
index ca1695835..fa96b2327 100644
--- a/gnu/packages/cups.scm
+++ b/gnu/packages/cups.scm
@@ -3,6 +3,7 @@
 ;;; Copyright © 2015, 2016 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2015, 2016 Efraim Flashner <efraim@flashner.co.il>
 ;;; Copyright © 2016 Danny Milosavljevic <dannym@scratchpost.org>
+;;; Copyright © 2017 Leo Famulari <leo@famulari.name>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -51,6 +52,7 @@
 (define-public cups-filters
   (package
     (name "cups-filters")
+    (replacement cups-filters/fixed)
     (version "1.13.1")
     (source(origin
               (method url-fetch)
@@ -133,6 +135,12 @@ filters for the PDF-centric printing workflow introduced by OpenPrinting.")
                    license:lgpl2.0+
                    license:expat))))
 
+(define mupdf/fixed-instead-of-mupdf
+  (package-input-rewriting `((,mupdf . ,(@@ (gnu packages pdf) mupdf/fixed)))))
+
+(define cups-filters/fixed
+  (mupdf/fixed-instead-of-mupdf cups-filters))
+
 ;; CUPS on non-MacOS systems requires cups-filters.  Since cups-filters also
 ;; depends on CUPS libraries and binaries, cups-minimal has been added to
 ;; satisfy this dependency.
-- 
2.11.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH] gnu: mupdf: Fix some security problems in bundled mujs.
  2017-01-15 20:49             ` Leo Famulari
@ 2017-01-15 20:56               ` Marius Bakke
  2017-01-15 23:05               ` Mark H Weaver
  1 sibling, 0 replies; 13+ messages in thread
From: Marius Bakke @ 2017-01-15 20:56 UTC (permalink / raw)
  To: Leo Famulari; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 434 bytes --]

Leo Famulari <leo@famulari.name> writes:

> On Sun, Jan 15, 2017 at 08:05:48PM +0100, Marius Bakke wrote:
>> Is it possible to use the 'package-input-rewriting' procedure here? See
>> example at the end of section 5.1.0:
>> 
>> https://www.gnu.org/software/guix/manual/guix.html#Defining-Packages
>> 
>> Otherwise this LGTM, thanks a lot!
>
> Okay, please test this updated patch series :)

That looks great, thanks! :-)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] gnu: mupdf: Fix some security problems in bundled mujs.
  2017-01-15 20:49             ` Leo Famulari
  2017-01-15 20:56               ` Marius Bakke
@ 2017-01-15 23:05               ` Mark H Weaver
  2017-01-16  1:27                 ` Leo Famulari
  1 sibling, 1 reply; 13+ messages in thread
From: Mark H Weaver @ 2017-01-15 23:05 UTC (permalink / raw)
  To: Leo Famulari; +Cc: guix-devel

Hi Leo,

Leo Famulari <leo@famulari.name> writes:

> From 34cc0dc9d9451d540f8733ebca2a3db54a073aa0 Mon Sep 17 00:00:00 2001
> From: Marius Bakke <mbakke@fastmail.com>
> Date: Thu, 12 Jan 2017 19:06:55 +0100
> Subject: [PATCH 1/2] gnu: mupdf: Fix CVE-2016-{10132,10133} in bundled mujs.
>
> * gnu/packages/patches/mupdf-mujs-CVE-2016-10132.patch,
> gnu/packages/patches/mupdf-mujs-CVE-2016-10133.patch: New files.
> * gnu/local.mk (dist_patch_DATA): Add them.
> * gnu/packages/pdf.scm (mupdf)[replacement]: New field.

We should indeed add a 'replacement' field to 'mupdf', but that part of
the patch seems to have gotten lost:

> diff --git a/gnu/packages/pdf.scm b/gnu/packages/pdf.scm
> index 9b3571e67..5efc5e6d1 100644
> --- a/gnu/packages/pdf.scm
> +++ b/gnu/packages/pdf.scm
> @@ -6,10 +6,11 @@
>  ;;; Copyright © 2016 Roel Janssen <roel@gnu.org>
>  ;;; Coypright © 2016 ng0 <ng0@we.make.ritual.n0.is>
>  ;;; Coypright © 2016 Efraim Flashner <efraim@flashner.co.il>
> -;;; Coypright © 2016 Marius Bakke <mbakke@fastmail.com>
> +;;; Coypright © 2016, 2017 Marius Bakke <mbakke@fastmail.com>
>  ;;; Coypright © 2016 Ludovic Courtès <ludo@gnu.org>
>  ;;; Coypright © 2016 Julien Lepiller <julien@lepiller.eu>
>  ;;; Copyright © 2016 Arun Isaac <arunisaac@systemreboot.net>
> +;;; Copyright © 2017 Leo Famulari <leo@famulari.name>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -538,6 +539,18 @@ line tools for batch rendering (pdfdraw), rewriting files (pdfclean),
>  and examining the file structure (pdfshow).")
>      (license license:agpl3+)))
>  
> +(define mupdf/fixed
> +  (package
> +    (inherit mupdf)
> +    (source
> +      (origin
> +        (inherit (package-source mupdf))
> +        (patches
> +          (append
> +            (origin-patches (package-source mupdf))
> +            (search-patches "mupdf-mujs-CVE-2016-10132.patch"
> +                            "mupdf-mujs-CVE-2016-10133.patch")))))))
> +
>  (define-public qpdf
>    (package
>     (name "qpdf")

Also, you should probably add a "Co-authored-by:" header in the commit
log for yourself :)

Otherwise it looks good to me.
Thanks to both of you for working on it!

      Mark

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] gnu: mupdf: Fix some security problems in bundled mujs.
  2017-01-15 23:05               ` Mark H Weaver
@ 2017-01-16  1:27                 ` Leo Famulari
  0 siblings, 0 replies; 13+ messages in thread
From: Leo Famulari @ 2017-01-16  1:27 UTC (permalink / raw)
  To: Mark H Weaver; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 955 bytes --]

On Sun, Jan 15, 2017 at 06:05:02PM -0500, Mark H Weaver wrote:
> Hi Leo,
> 
> Leo Famulari <leo@famulari.name> writes:
> 
> > From 34cc0dc9d9451d540f8733ebca2a3db54a073aa0 Mon Sep 17 00:00:00 2001
> > From: Marius Bakke <mbakke@fastmail.com>
> > Date: Thu, 12 Jan 2017 19:06:55 +0100
> > Subject: [PATCH 1/2] gnu: mupdf: Fix CVE-2016-{10132,10133} in bundled mujs.
> >
> > * gnu/packages/patches/mupdf-mujs-CVE-2016-10132.patch,
> > gnu/packages/patches/mupdf-mujs-CVE-2016-10133.patch: New files.
> > * gnu/local.mk (dist_patch_DATA): Add them.
> > * gnu/packages/pdf.scm (mupdf)[replacement]: New field.
> 
> We should indeed add a 'replacement' field to 'mupdf', but that part of
> the patch seems to have gotten lost:

I suspected something was wrong. Thank you for catching this.

And thanks to Marius as well! I'm happy to be collaborating on these
commits.

I pushed the changes as 8afabb2eca954af6fbba8c6ae37e8f0bc3047840.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2017-01-16  1:27 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-12 18:06 [PATCH] gnu: mupdf: Fix some security problems in bundled mujs Marius Bakke
2017-01-12 18:30 ` Leo Famulari
2017-01-12 19:46   ` Marius Bakke
2017-01-12 20:03     ` Leo Famulari
2017-01-13  0:59       ` Mark H Weaver
2017-01-13 17:34         ` Leo Famulari
2017-01-15  8:20           ` Mark H Weaver
2017-01-15 18:47         ` Leo Famulari
2017-01-15 19:05           ` Marius Bakke
2017-01-15 20:49             ` Leo Famulari
2017-01-15 20:56               ` Marius Bakke
2017-01-15 23:05               ` Mark H Weaver
2017-01-16  1:27                 ` Leo Famulari

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).