* [PATCH 0/1] Gst-plugins-good security update
@ 2016-11-25 7:11 Leo Famulari
2016-11-25 7:11 ` [PATCH 1/1] gnu: gst-plugins-good: Fix CVE-2016-{9634,9635,9636} Leo Famulari
2016-11-26 8:51 ` [PATCH 0/1] Gst-plugins-good security update Marius Bakke
0 siblings, 2 replies; 6+ messages in thread
From: Leo Famulari @ 2016-11-25 7:11 UTC (permalink / raw)
To: guix-devel
This patch should fix the bugs named here:
http://seclists.org/oss-sec/2016/q4/517
I copied Debian's approach, which is to take all the recent patches for
the vulnerable component (the FLIC decoder).
My understanding is that the first two patches fix the CVEs, the 3rd
fixes an unrelated bug, and the 4th is a total rewrite of the component,
because "code is terrible, it should be entirely re-written" [0].
The CVE bug fixes are not split into discrete patches, so it doesn't
work to make patches for each CVE ID, like we normally do.
Is this approach (concatenating the patches) okay?
[0]
https://bugzilla.gnome.org/show_bug.cgi?id=774859#c1
Leo Famulari (1):
gnu: gst-plugins-good: Fix CVE-2016-{9634,9635,9636}.
gnu/local.mk | 1 +
gnu/packages/gstreamer.scm | 1 +
.../gst-plugins-good-flxdec-heap-overflow.patch | 1433 ++++++++++++++++++++
3 files changed, 1435 insertions(+)
create mode 100644 gnu/packages/patches/gst-plugins-good-flxdec-heap-overflow.patch
--
2.10.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/1] gnu: gst-plugins-good: Fix CVE-2016-{9634,9635,9636}.
2016-11-25 7:11 [PATCH 0/1] Gst-plugins-good security update Leo Famulari
@ 2016-11-25 7:11 ` Leo Famulari
2016-11-26 8:51 ` [PATCH 0/1] Gst-plugins-good security update Marius Bakke
1 sibling, 0 replies; 6+ messages in thread
From: Leo Famulari @ 2016-11-25 7:11 UTC (permalink / raw)
To: guix-devel
* gnu/packages/patches/gst-plugins-good-flxdec-heap-overflow.patch: New file.
* gnu/local.mk (dist_patch_DATA): Add it.
* gnu/packages/gstreamer.scm (gst-plugins-good): Use it.
---
gnu/local.mk | 1 +
gnu/packages/gstreamer.scm | 1 +
.../gst-plugins-good-flxdec-heap-overflow.patch | 1433 ++++++++++++++++++++
3 files changed, 1435 insertions(+)
create mode 100644 gnu/packages/patches/gst-plugins-good-flxdec-heap-overflow.patch
diff --git a/gnu/local.mk b/gnu/local.mk
index 4913727..885e393 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -585,6 +585,7 @@ dist_patch_DATA = \
%D%/packages/patches/grub-gets-undeclared.patch \
%D%/packages/patches/grub-freetype.patch \
%D%/packages/patches/gsl-test-i686.patch \
+ %D%/packages/patches/gst-plugins-good-flxdec-heap-overflow.patch \
%D%/packages/patches/guile-1.8-cpp-4.5.patch \
%D%/packages/patches/guile-arm-fixes.patch \
%D%/packages/patches/guile-default-utf8.patch \
diff --git a/gnu/packages/gstreamer.scm b/gnu/packages/gstreamer.scm
index 5fe84ec..6c59ca2 100644
--- a/gnu/packages/gstreamer.scm
+++ b/gnu/packages/gstreamer.scm
@@ -207,6 +207,7 @@ for the GStreamer multimedia library.")
(uri (string-append
"https://gstreamer.freedesktop.org/src/" name "/"
name "-" version ".tar.xz"))
+ (patches (search-patches "gst-plugins-good-flxdec-heap-overflow.patch"))
(sha256
(base32
"1hkcap9l2603266gyi6jgvx7frbvfmb7xhfhjizbczy1wykjwr57"))))
diff --git a/gnu/packages/patches/gst-plugins-good-flxdec-heap-overflow.patch b/gnu/packages/patches/gst-plugins-good-flxdec-heap-overflow.patch
new file mode 100644
index 0000000..d4d483b
--- /dev/null
+++ b/gnu/packages/patches/gst-plugins-good-flxdec-heap-overflow.patch
@@ -0,0 +1,1433 @@
+These 4 patches fix some security issues in gst-plugins-good's FLIC
+decoder, including CVE-2016-{9634,9635,9636}.
+
+https://security-tracker.debian.org/tracker/DSA-3723-1
+https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2016-9634
+https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2016-9635
+https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2016-9636
+
+All 4 patches are copied from the upstream source repository:
+
+https://cgit.freedesktop.org/gstreamer/gst-plugins-good/
+
+Each patch includes a link to the relevant upstream bug report.
+
+From 2e203a79b7d9af4029307c1a845b3c148d5f5e62 Mon Sep 17 00:00:00 2001
+From: Matthew Waters <matthew@centricular.com>
+Date: Tue, 22 Nov 2016 19:05:00 +1100
+Subject: [PATCH] flxdec: add some write bounds checking
+
+Without checking the bounds of the frame we are writing into, we can
+write off the end of the destination buffer.
+
+https://scarybeastsecurity.blogspot.dk/2016/11/0day-exploit-advancing-exploitation.html
+
+https://bugzilla.gnome.org/show_bug.cgi?id=774834
+---
+ gst/flx/gstflxdec.c | 116 +++++++++++++++++++++++++++++++++++++++++-----------
+ 1 file changed, 91 insertions(+), 25 deletions(-)
+
+diff --git a/gst/flx/gstflxdec.c b/gst/flx/gstflxdec.c
+index 604be2f..d51a8e6 100644
+--- a/gst/flx/gstflxdec.c
++++ b/gst/flx/gstflxdec.c
+@@ -74,9 +74,9 @@ static gboolean gst_flxdec_src_query_handler (GstPad * pad, GstObject * parent,
+ GstQuery * query);
+
+ static void flx_decode_color (GstFlxDec *, guchar *, guchar *, gint);
+-static void flx_decode_brun (GstFlxDec *, guchar *, guchar *);
+-static void flx_decode_delta_fli (GstFlxDec *, guchar *, guchar *);
+-static void flx_decode_delta_flc (GstFlxDec *, guchar *, guchar *);
++static gboolean flx_decode_brun (GstFlxDec *, guchar *, guchar *);
++static gboolean flx_decode_delta_fli (GstFlxDec *, guchar *, guchar *);
++static gboolean flx_decode_delta_flc (GstFlxDec *, guchar *, guchar *);
+
+ #define rndalign(off) ((off) + ((off) & 1))
+
+@@ -203,13 +203,14 @@ gst_flxdec_sink_event_handler (GstPad * pad, GstObject * parent,
+ return ret;
+ }
+
+-static void
++static gboolean
+ flx_decode_chunks (GstFlxDec * flxdec, gulong count, guchar * data,
+ guchar * dest)
+ {
+ FlxFrameChunk *hdr;
++ gboolean ret = TRUE;
+
+- g_return_if_fail (data != NULL);
++ g_return_val_if_fail (data != NULL, FALSE);
+
+ while (count--) {
+ hdr = (FlxFrameChunk *) data;
+@@ -228,17 +229,17 @@ flx_decode_chunks (GstFlxDec * flxdec, gulong count, guchar * data,
+ break;
+
+ case FLX_BRUN:
+- flx_decode_brun (flxdec, data, dest);
++ ret = flx_decode_brun (flxdec, data, dest);
+ data += rndalign (hdr->size) - FlxFrameChunkSize;
+ break;
+
+ case FLX_LC:
+- flx_decode_delta_fli (flxdec, data, dest);
++ ret = flx_decode_delta_fli (flxdec, data, dest);
+ data += rndalign (hdr->size) - FlxFrameChunkSize;
+ break;
+
+ case FLX_SS2:
+- flx_decode_delta_flc (flxdec, data, dest);
++ ret = flx_decode_delta_flc (flxdec, data, dest);
+ data += rndalign (hdr->size) - FlxFrameChunkSize;
+ break;
+
+@@ -256,7 +257,12 @@ flx_decode_chunks (GstFlxDec * flxdec, gulong count, guchar * data,
+ data += rndalign (hdr->size) - FlxFrameChunkSize;
+ break;
+ }
++
++ if (!ret)
++ break;
+ }
++
++ return ret;
+ }
+
+
+@@ -289,13 +295,13 @@ flx_decode_color (GstFlxDec * flxdec, guchar * data, guchar * dest, gint scale)
+ }
+ }
+
+-static void
++static gboolean
+ flx_decode_brun (GstFlxDec * flxdec, guchar * data, guchar * dest)
+ {
+ gulong count, lines, row;
+ guchar x;
+
+- g_return_if_fail (flxdec != NULL);
++ g_return_val_if_fail (flxdec != NULL, FALSE);
+
+ lines = flxdec->hdr.height;
+ while (lines--) {
+@@ -313,12 +319,21 @@ flx_decode_brun (GstFlxDec * flxdec, guchar * data, guchar * dest)
+ if (count > 0x7f) {
+ /* literal run */
+ count = 0x100 - count;
++ if ((glong) row - count < 0) {
++ GST_ERROR_OBJECT (flxdec, "Invalid BRUN packet detected.");
++ return FALSE;
++ }
+ row -= count;
+
+ while (count--)
+ *dest++ = *data++;
+
+ } else {
++ if ((glong) row - count < 0) {
++ GST_ERROR_OBJECT (flxdec, "Invalid BRUN packet detected.");
++ return FALSE;
++ }
++
+ /* replicate run */
+ row -= count;
+ x = *data++;
+@@ -328,22 +343,28 @@ flx_decode_brun (GstFlxDec * flxdec, guchar * data, guchar * dest)
+ }
+ }
+ }
++
++ return TRUE;
+ }
+
+-static void
++static gboolean
+ flx_decode_delta_fli (GstFlxDec * flxdec, guchar * data, guchar * dest)
+ {
+ gulong count, packets, lines, start_line;
+ guchar *start_p, x;
+
+- g_return_if_fail (flxdec != NULL);
+- g_return_if_fail (flxdec->delta_data != NULL);
++ g_return_val_if_fail (flxdec != NULL, FALSE);
++ g_return_val_if_fail (flxdec->delta_data != NULL, FALSE);
+
+ /* use last frame for delta */
+ memcpy (dest, flxdec->delta_data, flxdec->size);
+
+ start_line = (data[0] + (data[1] << 8));
+ lines = (data[2] + (data[3] << 8));
++ if (start_line + lines > flxdec->hdr.height) {
++ GST_ERROR_OBJECT (flxdec, "Invalid FLI packet detected. too many lines.");
++ return FALSE;
++ }
+ data += 4;
+
+ /* start position of delta */
+@@ -356,7 +377,8 @@ flx_decode_delta_fli (GstFlxDec * flxdec, guchar * data, guchar * dest)
+
+ while (packets--) {
+ /* skip count */
+- dest += *data++;
++ guchar skip = *data++;
++ dest += skip;
+
+ /* RLE count */
+ count = *data++;
+@@ -364,12 +386,24 @@ flx_decode_delta_fli (GstFlxDec * flxdec, guchar * data, guchar * dest)
+ if (count > 0x7f) {
+ /* literal run */
+ count = 0x100 - count;
+- x = *data++;
+
++ if (skip + count > flxdec->hdr.width) {
++ GST_ERROR_OBJECT (flxdec, "Invalid FLI packet detected. "
++ "line too long.");
++ return FALSE;
++ }
++
++ x = *data++;
+ while (count--)
+ *dest++ = x;
+
+ } else {
++ if (skip + count > flxdec->hdr.width) {
++ GST_ERROR_OBJECT (flxdec, "Invalid FLI packet detected. "
++ "line too long.");
++ return FALSE;
++ }
++
+ /* replicate run */
+ while (count--)
+ *dest++ = *data++;
+@@ -378,21 +412,27 @@ flx_decode_delta_fli (GstFlxDec * flxdec, guchar * data, guchar * dest)
+ start_p += flxdec->hdr.width;
+ dest = start_p;
+ }
++
++ return TRUE;
+ }
+
+-static void
++static gboolean
+ flx_decode_delta_flc (GstFlxDec * flxdec, guchar * data, guchar * dest)
+ {
+ gulong count, lines, start_l, opcode;
+ guchar *start_p;
+
+- g_return_if_fail (flxdec != NULL);
+- g_return_if_fail (flxdec->delta_data != NULL);
++ g_return_val_if_fail (flxdec != NULL, FALSE);
++ g_return_val_if_fail (flxdec->delta_data != NULL, FALSE);
+
+ /* use last frame for delta */
+ memcpy (dest, flxdec->delta_data, flxdec->size);
+
+ lines = (data[0] + (data[1] << 8));
++ if (lines > flxdec->hdr.height) {
++ GST_ERROR_OBJECT (flxdec, "Invalid FLC packet detected. too many lines.");
++ return FALSE;
++ }
+ data += 2;
+
+ start_p = dest;
+@@ -405,9 +445,15 @@ flx_decode_delta_flc (GstFlxDec * flxdec, guchar * data, guchar * dest)
+ while ((opcode = (data[0] + (data[1] << 8))) & 0xc000) {
+ data += 2;
+ if ((opcode & 0xc000) == 0xc000) {
+- /* skip count */
+- start_l += (0x10000 - opcode);
+- dest += flxdec->hdr.width * (0x10000 - opcode);
++ /* line skip count */
++ gulong skip = (0x10000 - opcode);
++ if (skip > flxdec->hdr.height) {
++ GST_ERROR_OBJECT (flxdec, "Invalid FLC packet detected. "
++ "skip line count too big.");
++ return FALSE;
++ }
++ start_l += skip;
++ dest += flxdec->hdr.width * skip;
+ } else {
+ /* last pixel */
+ dest += flxdec->hdr.width;
+@@ -419,7 +465,8 @@ flx_decode_delta_flc (GstFlxDec * flxdec, guchar * data, guchar * dest)
+ /* last opcode is the packet count */
+ while (opcode--) {
+ /* skip count */
+- dest += *data++;
++ guchar skip = *data++;
++ dest += skip;
+
+ /* RLE count */
+ count = *data++;
+@@ -427,12 +474,25 @@ flx_decode_delta_flc (GstFlxDec * flxdec, guchar * data, guchar * dest)
+ if (count > 0x7f) {
+ /* replicate word run */
+ count = 0x100 - count;
++
++ if (skip + count > flxdec->hdr.width) {
++ GST_ERROR_OBJECT (flxdec, "Invalid FLC packet detected. "
++ "line too long.");
++ return FALSE;
++ }
++
+ while (count--) {
+ *dest++ = data[0];
+ *dest++ = data[1];
+ }
+ data += 2;
+ } else {
++ if (skip + count > flxdec->hdr.width) {
++ GST_ERROR_OBJECT (flxdec, "Invalid FLC packet detected. "
++ "line too long.");
++ return FALSE;
++ }
++
+ /* literal word run */
+ while (count--) {
+ *dest++ = *data++;
+@@ -442,6 +502,8 @@ flx_decode_delta_flc (GstFlxDec * flxdec, guchar * data, guchar * dest)
+ }
+ lines--;
+ }
++
++ return TRUE;
+ }
+
+ static GstFlowReturn
+@@ -571,9 +633,13 @@ gst_flxdec_chain (GstPad * pad, GstObject * parent, GstBuffer * buf)
+ out = gst_buffer_new_and_alloc (flxdec->size * 4);
+
+ /* decode chunks */
+- flx_decode_chunks (flxdec,
+- ((FlxFrameType *) chunk)->chunks,
+- chunk + FlxFrameTypeSize, flxdec->frame_data);
++ if (!flx_decode_chunks (flxdec,
++ ((FlxFrameType *) chunk)->chunks,
++ chunk + FlxFrameTypeSize, flxdec->frame_data)) {
++ GST_ELEMENT_ERROR (flxdec, STREAM, DECODE,
++ ("%s", "Could not decode chunk"), NULL);
++ return GST_FLOW_ERROR;
++ }
+
+ /* save copy of the current frame for possible delta. */
+ memcpy (flxdec->delta_data, flxdec->frame_data, flxdec->size);
+--
+2.10.2
+
+From 1ab2b26193861b124426e2f8eb62b75b59ec5488 Mon Sep 17 00:00:00 2001
+From: Matthew Waters <matthew@centricular.com>
+Date: Tue, 22 Nov 2016 23:46:00 +1100
+Subject: [PATCH] flxdec: fix some warnings comparing unsigned < 0
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+bf43f44fcfada5ec4a3ce60cb374340486fe9fac was comparing an unsigned
+expression to be < 0 which was always false.
+
+gstflxdec.c: In function ‘flx_decode_brun’:
+gstflxdec.c:322:33: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
+ if ((glong) row - count < 0) {
+ ^
+gstflxdec.c:332:33: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
+ if ((glong) row - count < 0) {
+ ^
+
+https://bugzilla.gnome.org/show_bug.cgi?id=774834
+---
+ gst/flx/gstflxdec.c | 4 ++--
+ 1 file changed, 2 insertions(+), 2 deletions(-)
+
+diff --git a/gst/flx/gstflxdec.c b/gst/flx/gstflxdec.c
+index d51a8e6..e675c99 100644
+--- a/gst/flx/gstflxdec.c
++++ b/gst/flx/gstflxdec.c
+@@ -319,7 +319,7 @@ flx_decode_brun (GstFlxDec * flxdec, guchar * data, guchar * dest)
+ if (count > 0x7f) {
+ /* literal run */
+ count = 0x100 - count;
+- if ((glong) row - count < 0) {
++ if ((glong) row - (glong) count < 0) {
+ GST_ERROR_OBJECT (flxdec, "Invalid BRUN packet detected.");
+ return FALSE;
+ }
+@@ -329,7 +329,7 @@ flx_decode_brun (GstFlxDec * flxdec, guchar * data, guchar * dest)
+ *dest++ = *data++;
+
+ } else {
+- if ((glong) row - count < 0) {
++ if ((glong) row - (glong) count < 0) {
+ GST_ERROR_OBJECT (flxdec, "Invalid BRUN packet detected.");
+ return FALSE;
+ }
+--
+2.10.2
+
+From b31c504645a814c59d91d49e4fe218acaf93f4ca Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= <sebastian@centricular.com>
+Date: Wed, 23 Nov 2016 11:20:49 +0200
+Subject: [PATCH] flxdec: Don't unref() parent in the chain function
+
+We don't own the reference here, it is owned by the caller and given to
+us for the scope of this function. Leftover mistake from 0.10 porting.
+
+https://bugzilla.gnome.org/show_bug.cgi?id=774897
+---
+ gst/flx/gstflxdec.c | 1 -
+ 1 file changed, 1 deletion(-)
+
+diff --git a/gst/flx/gstflxdec.c b/gst/flx/gstflxdec.c
+index e675c99..a237976 100644
+--- a/gst/flx/gstflxdec.c
++++ b/gst/flx/gstflxdec.c
+@@ -677,7 +677,6 @@ wrong_type:
+ {
+ GST_ELEMENT_ERROR (flxdec, STREAM, WRONG_TYPE, (NULL),
+ ("not a flx file (type %x)", flxh->type));
+- gst_object_unref (flxdec);
+ return GST_FLOW_ERROR;
+ }
+ }
+--
+2.10.2
+
+From be670f0daf67304fb92c76aa09c30cae0bfd1fe4 Mon Sep 17 00:00:00 2001
+From: Matthew Waters <matthew@centricular.com>
+Date: Wed, 23 Nov 2016 07:09:06 +1100
+Subject: [PATCH] flxdec: rewrite logic based on GstByteReader/Writer
+
+Solves overreading/writing the given arrays and will error out if the
+streams asks to do that.
+
+Also does more error checking that the stream is valid and won't
+overrun any allocated arrays. Also mitigate integer overflow errors
+calculating allocation sizes.
+
+https://bugzilla.gnome.org/show_bug.cgi?id=774859
+---
+ gst/flx/flx_color.c | 1 -
+ gst/flx/flx_fmt.h | 72 -------
+ gst/flx/gstflxdec.c | 610 ++++++++++++++++++++++++++++++++++++----------------
+ gst/flx/gstflxdec.h | 4 +-
+ 4 files changed, 427 insertions(+), 260 deletions(-)
+
+diff --git a/gst/flx/flx_color.c b/gst/flx/flx_color.c
+index 047bfdf..3a58135 100644
+--- a/gst/flx/flx_color.c
++++ b/gst/flx/flx_color.c
+@@ -101,7 +101,6 @@ flx_set_palette_vector (FlxColorSpaceConverter * flxpal, guint start, guint num,
+ } else {
+ memcpy (&flxpal->palvec[start * 3], newpal, grab * 3);
+ }
+-
+ }
+
+ void
+diff --git a/gst/flx/flx_fmt.h b/gst/flx/flx_fmt.h
+index 9ab31ba..abff200 100644
+--- a/gst/flx/flx_fmt.h
++++ b/gst/flx/flx_fmt.h
+@@ -123,78 +123,6 @@ typedef struct _FlxFrameType
+ } FlxFrameType;
+ #define FlxFrameTypeSize 10
+
+-#if G_BYTE_ORDER == G_BIG_ENDIAN
+-#define LE_TO_BE_16(i16) ((guint16) (((i16) << 8) | ((i16) >> 8)))
+-#define LE_TO_BE_32(i32) \
+- (((guint32) (LE_TO_BE_16((guint16) (i32))) << 16) | (LE_TO_BE_16((i32) >> 16)))
+-
+-#define FLX_FRAME_TYPE_FIX_ENDIANNESS(frm_type_p) \
+- do { \
+- (frm_type_p)->chunks = LE_TO_BE_16((frm_type_p)->chunks); \
+- (frm_type_p)->delay = LE_TO_BE_16((frm_type_p)->delay); \
+- } while(0)
+-
+-#define FLX_HUFFMAN_TABLE_FIX_ENDIANNESS(hffmn_table_p) \
+- do { \
+- (hffmn_table_p)->codelength = \
+- LE_TO_BE_16((hffmn_table_p)->codelength); \
+- (hffmn_table_p)->numcodes = LE_TO_BE_16((hffmn_table_p)->numcodes); \
+- } while(0)
+-
+-#define FLX_SEGMENT_TABLE_FIX_ENDIANNESS(sgmnt_table_p) \
+- ((sgmnt_table_p)->segments = LE_TO_BE_16((sgmnt_table_p)->segments))
+-
+-#define FLX_PREFIX_CHUNK_FIX_ENDIANNESS(prfx_chnk_p) \
+- do { \
+- (prfx_chnk_p)->chunks = LE_TO_BE_16((prfx_chnk_p)->chunks); \
+- } while(0)
+-
+-#define FLX_FRAME_CHUNK_FIX_ENDIANNESS(frm_chnk_p) \
+- do { \
+- (frm_chnk_p)->size = LE_TO_BE_32((frm_chnk_p)->size); \
+- (frm_chnk_p)->id = LE_TO_BE_16((frm_chnk_p)->id); \
+- } while(0)
+-
+-#define FLX_HDR_FIX_ENDIANNESS(hdr_p) \
+- do { \
+- (hdr_p)->size = LE_TO_BE_32((hdr_p)->size); \
+- (hdr_p)->type = LE_TO_BE_16((hdr_p)->type); \
+- (hdr_p)->frames = LE_TO_BE_16((hdr_p)->frames); \
+- (hdr_p)->width = LE_TO_BE_16((hdr_p)->width); \
+- (hdr_p)->height = LE_TO_BE_16((hdr_p)->height); \
+- (hdr_p)->depth = LE_TO_BE_16((hdr_p)->depth); \
+- (hdr_p)->flags = LE_TO_BE_16((hdr_p)->flags); \
+- (hdr_p)->speed = LE_TO_BE_32((hdr_p)->speed); \
+- (hdr_p)->reserved1 = LE_TO_BE_16((hdr_p)->reserved1); \
+- (hdr_p)->created = LE_TO_BE_32((hdr_p)->created); \
+- (hdr_p)->creator = LE_TO_BE_32((hdr_p)->creator); \
+- (hdr_p)->updated = LE_TO_BE_32((hdr_p)->updated); \
+- (hdr_p)->updater = LE_TO_BE_32((hdr_p)->updater); \
+- (hdr_p)->aspect_dx = LE_TO_BE_16((hdr_p)->aspect_dx); \
+- (hdr_p)->aspect_dy = LE_TO_BE_16((hdr_p)->aspect_dy); \
+- (hdr_p)->ext_flags = LE_TO_BE_16((hdr_p)->ext_flags); \
+- (hdr_p)->keyframes = LE_TO_BE_16((hdr_p)->keyframes); \
+- (hdr_p)->totalframes = LE_TO_BE_16((hdr_p)->totalframes); \
+- (hdr_p)->req_memory = LE_TO_BE_32((hdr_p)->req_memory); \
+- (hdr_p)->max_regions = LE_TO_BE_16((hdr_p)->max_regions); \
+- (hdr_p)->transp_num = LE_TO_BE_16((hdr_p)->transp_num); \
+- (hdr_p)->oframe1 = LE_TO_BE_32((hdr_p)->oframe1); \
+- (hdr_p)->oframe2 = LE_TO_BE_32((hdr_p)->oframe2); \
+- } while(0)
+-#else
+-
+-#define LE_TO_BE_16(i16) ((i16))
+-#define LE_TO_BE_32(i32) ((i32))
+-
+-#define FLX_FRAME_TYPE_FIX_ENDIANNESS(frm_type_p)
+-#define FLX_HUFFMAN_TABLE_FIX_ENDIANNESS(hffmn_table_p)
+-#define FLX_SEGMENT_TABLE_FIX_ENDIANNESS(sgmnt_table_p)
+-#define FLX_PREFIX_CHUNK_FIX_ENDIANNESS(prfx_chnk_p)
+-#define FLX_FRAME_CHUNK_FIX_ENDIANNESS(frm_chnk_p)
+-#define FLX_HDR_FIX_ENDIANNESS(hdr_p)
+-
+-#endif /* G_BYTE_ORDER == G_BIG_ENDIAN */
+-
+ G_END_DECLS
+
+ #endif /* __GST_FLX_FMT_H__ */
+diff --git a/gst/flx/gstflxdec.c b/gst/flx/gstflxdec.c
+index a237976..aa1bed5 100644
+--- a/gst/flx/gstflxdec.c
++++ b/gst/flx/gstflxdec.c
+@@ -1,5 +1,6 @@
+ /* GStreamer
+ * Copyright (C) <1999> Erik Walthinsen <omega@temple-baptist.com>
++ * Copyright (C) <2016> Matthew Waters <matthew@centricular.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Library General Public
+@@ -24,6 +25,7 @@
+ /*
+ * http://www.coolutils.com/Formats/FLI
+ * http://woodshole.er.usgs.gov/operations/modeling/flc.html
++ * http://www.compuphase.com/flic.htm
+ */
+
+ #ifdef HAVE_CONFIG_H
+@@ -73,10 +75,14 @@ static GstStateChangeReturn gst_flxdec_change_state (GstElement * element,
+ static gboolean gst_flxdec_src_query_handler (GstPad * pad, GstObject * parent,
+ GstQuery * query);
+
+-static void flx_decode_color (GstFlxDec *, guchar *, guchar *, gint);
+-static gboolean flx_decode_brun (GstFlxDec *, guchar *, guchar *);
+-static gboolean flx_decode_delta_fli (GstFlxDec *, guchar *, guchar *);
+-static gboolean flx_decode_delta_flc (GstFlxDec *, guchar *, guchar *);
++static gboolean flx_decode_color (GstFlxDec * flxdec, GstByteReader * reader,
++ GstByteWriter * writer, gint scale);
++static gboolean flx_decode_brun (GstFlxDec * flxdec,
++ GstByteReader * reader, GstByteWriter * writer);
++static gboolean flx_decode_delta_fli (GstFlxDec * flxdec,
++ GstByteReader * reader, GstByteWriter * writer);
++static gboolean flx_decode_delta_flc (GstFlxDec * flxdec,
++ GstByteReader * reader, GstByteWriter * writer);
+
+ #define rndalign(off) ((off) + ((off) & 1))
+
+@@ -204,57 +210,59 @@ gst_flxdec_sink_event_handler (GstPad * pad, GstObject * parent,
+ }
+
+ static gboolean
+-flx_decode_chunks (GstFlxDec * flxdec, gulong count, guchar * data,
+- guchar * dest)
++flx_decode_chunks (GstFlxDec * flxdec, gulong n_chunks, GstByteReader * reader,
++ GstByteWriter * writer)
+ {
+- FlxFrameChunk *hdr;
+ gboolean ret = TRUE;
+
+- g_return_val_if_fail (data != NULL, FALSE);
+-
+- while (count--) {
+- hdr = (FlxFrameChunk *) data;
+- FLX_FRAME_CHUNK_FIX_ENDIANNESS (hdr);
+- data += FlxFrameChunkSize;
++ while (n_chunks--) {
++ GstByteReader chunk;
++ guint32 size;
++ guint16 type;
++
++ if (!gst_byte_reader_get_uint32_le (reader, &size))
++ goto parse_error;
++ if (!gst_byte_reader_get_uint16_le (reader, &type))
++ goto parse_error;
++ GST_LOG_OBJECT (flxdec, "chunk has type 0x%02x size %d", type, size);
++
++ if (!gst_byte_reader_get_sub_reader (reader, &chunk,
++ size - FlxFrameChunkSize)) {
++ GST_ERROR_OBJECT (flxdec, "Incorrect size in the chunk header");
++ goto error;
++ }
+
+- switch (hdr->id) {
++ switch (type) {
+ case FLX_COLOR64:
+- flx_decode_color (flxdec, data, dest, 2);
+- data += rndalign (hdr->size) - FlxFrameChunkSize;
++ ret = flx_decode_color (flxdec, &chunk, writer, 2);
+ break;
+
+ case FLX_COLOR256:
+- flx_decode_color (flxdec, data, dest, 0);
+- data += rndalign (hdr->size) - FlxFrameChunkSize;
++ ret = flx_decode_color (flxdec, &chunk, writer, 0);
+ break;
+
+ case FLX_BRUN:
+- ret = flx_decode_brun (flxdec, data, dest);
+- data += rndalign (hdr->size) - FlxFrameChunkSize;
++ ret = flx_decode_brun (flxdec, &chunk, writer);
+ break;
+
+ case FLX_LC:
+- ret = flx_decode_delta_fli (flxdec, data, dest);
+- data += rndalign (hdr->size) - FlxFrameChunkSize;
++ ret = flx_decode_delta_fli (flxdec, &chunk, writer);
+ break;
+
+ case FLX_SS2:
+- ret = flx_decode_delta_flc (flxdec, data, dest);
+- data += rndalign (hdr->size) - FlxFrameChunkSize;
++ ret = flx_decode_delta_flc (flxdec, &chunk, writer);
+ break;
+
+ case FLX_BLACK:
+- memset (dest, 0, flxdec->size);
++ ret = gst_byte_writer_fill (writer, 0, flxdec->size);
+ break;
+
+ case FLX_MINI:
+- data += rndalign (hdr->size) - FlxFrameChunkSize;
+ break;
+
+ default:
+- GST_WARNING ("Unimplented chunk type: 0x%02x size: %d - skipping",
+- hdr->id, hdr->size);
+- data += rndalign (hdr->size) - FlxFrameChunkSize;
++ GST_WARNING ("Unimplemented chunk type: 0x%02x size: %d - skipping",
++ type, size);
+ break;
+ }
+
+@@ -263,43 +271,60 @@ flx_decode_chunks (GstFlxDec * flxdec, gulong count, guchar * data,
+ }
+
+ return ret;
++
++parse_error:
++ GST_ERROR_OBJECT (flxdec, "Failed to decode chunk");
++error:
++ return FALSE;
+ }
+
+
+-static void
+-flx_decode_color (GstFlxDec * flxdec, guchar * data, guchar * dest, gint scale)
++static gboolean
++flx_decode_color (GstFlxDec * flxdec, GstByteReader * reader,
++ GstByteWriter * writer, gint scale)
+ {
+- guint packs, count, indx;
++ guint8 count, indx;
++ guint16 packs;
+
+- g_return_if_fail (flxdec != NULL);
+-
+- packs = (data[0] + (data[1] << 8));
+-
+- data += 2;
++ if (!gst_byte_reader_get_uint16_le (reader, &packs))
++ goto error;
+ indx = 0;
+
+- GST_LOG ("GstFlxDec: cmap packs: %d", packs);
++ GST_LOG ("GstFlxDec: cmap packs: %d", (guint) packs);
+ while (packs--) {
++ const guint8 *data;
++ guint16 actual_count;
++
+ /* color map index + skip count */
+- indx += *data++;
++ if (!gst_byte_reader_get_uint8 (reader, &indx))
++ goto error;
+
+ /* number of rgb triplets */
+- count = *data++ & 0xff;
+- if (count == 0)
+- count = 256;
++ if (!gst_byte_reader_get_uint8 (reader, &count))
++ goto error;
+
+- GST_LOG ("GstFlxDec: cmap count: %d (indx: %d)", count, indx);
+- flx_set_palette_vector (flxdec->converter, indx, count, data, scale);
++ actual_count = count == 0 ? 256 : count;
+
+- data += (count * 3);
++ if (!gst_byte_reader_get_data (reader, count * 3, &data))
++ goto error;
++
++ GST_LOG_OBJECT (flxdec, "cmap count: %d (indx: %d)", actual_count, indx);
++ flx_set_palette_vector (flxdec->converter, indx, actual_count,
++ (guchar *) data, scale);
+ }
++
++ return TRUE;
++
++error:
++ GST_ERROR_OBJECT (flxdec, "Error decoding color palette");
++ return FALSE;
+ }
+
+ static gboolean
+-flx_decode_brun (GstFlxDec * flxdec, guchar * data, guchar * dest)
++flx_decode_brun (GstFlxDec * flxdec, GstByteReader * reader,
++ GstByteWriter * writer)
+ {
+- gulong count, lines, row;
+- guchar x;
++ gulong lines, row;
+
+ g_return_val_if_fail (flxdec != NULL, FALSE);
+
+@@ -310,82 +335,125 @@ flx_decode_brun (GstFlxDec * flxdec, guchar * data, guchar * dest)
+ * contain more then 255 RLE packets. we use the frame
+ * width instead.
+ */
+- data++;
++ if (!gst_byte_reader_skip (reader, 1))
++ goto error;
+
+ row = flxdec->hdr.width;
+ while (row) {
+- count = *data++;
++ gint8 count;
++
++ if (!gst_byte_reader_get_int8 (reader, &count))
++ goto error;
++
++ if (count <= 0) {
++ const guint8 *data;
+
+- if (count > 0x7f) {
+ /* literal run */
+- count = 0x100 - count;
+- if ((glong) row - (glong) count < 0) {
+- GST_ERROR_OBJECT (flxdec, "Invalid BRUN packet detected.");
++ count = ABS (count);
++
++ GST_LOG_OBJECT (flxdec, "have literal run of size %d", count);
++
++ if (count > row) {
++ GST_ERROR_OBJECT (flxdec, "Invalid BRUN line detected. "
++ "bytes to write exceeds the end of the row");
+ return FALSE;
+ }
+ row -= count;
+
+- while (count--)
+- *dest++ = *data++;
+-
++ if (!gst_byte_reader_get_data (reader, count, &data))
++ goto error;
++ if (!gst_byte_writer_put_data (writer, data, count))
++ goto error;
+ } else {
+- if ((glong) row - (glong) count < 0) {
+- GST_ERROR_OBJECT (flxdec, "Invalid BRUN packet detected.");
++ guint8 x;
++
++ GST_LOG_OBJECT (flxdec, "have replicate run of size %d", count);
++
++ if (count > row) {
++ GST_ERROR_OBJECT (flxdec, "Invalid BRUN packet detected."
++ "bytes to write exceeds the end of the row");
+ return FALSE;
+ }
+
+ /* replicate run */
+ row -= count;
+- x = *data++;
+
+- while (count--)
+- *dest++ = x;
++ if (!gst_byte_reader_get_uint8 (reader, &x))
++ goto error;
++ if (!gst_byte_writer_fill (writer, x, count))
++ goto error;
+ }
+ }
+ }
+
+ return TRUE;
++
++error:
++ GST_ERROR_OBJECT (flxdec, "Failed to decode BRUN packet");
++ return FALSE;
+ }
+
+ static gboolean
+-flx_decode_delta_fli (GstFlxDec * flxdec, guchar * data, guchar * dest)
++flx_decode_delta_fli (GstFlxDec * flxdec, GstByteReader * reader,
++ GstByteWriter * writer)
+ {
+- gulong count, packets, lines, start_line;
+- guchar *start_p, x;
++ guint16 start_line, lines;
++ guint line_start_i;
+
+ g_return_val_if_fail (flxdec != NULL, FALSE);
+ g_return_val_if_fail (flxdec->delta_data != NULL, FALSE);
+
+ /* use last frame for delta */
+- memcpy (dest, flxdec->delta_data, flxdec->size);
++ if (!gst_byte_writer_put_data (writer, flxdec->delta_data, flxdec->size))
++ goto error;
++
++ if (!gst_byte_reader_get_uint16_le (reader, &start_line))
++ goto error;
++ if (!gst_byte_reader_get_uint16_le (reader, &lines))
++ goto error;
++ GST_LOG_OBJECT (flxdec, "height %d start line %d line count %d",
++ flxdec->hdr.height, start_line, lines);
+
+- start_line = (data[0] + (data[1] << 8));
+- lines = (data[2] + (data[3] << 8));
+ if (start_line + lines > flxdec->hdr.height) {
+ GST_ERROR_OBJECT (flxdec, "Invalid FLI packet detected. too many lines.");
+ return FALSE;
+ }
+- data += 4;
+
+- /* start position of delta */
+- dest += (flxdec->hdr.width * start_line);
+- start_p = dest;
++ line_start_i = flxdec->hdr.width * start_line;
++ if (!gst_byte_writer_set_pos (writer, line_start_i))
++ goto error;
+
+ while (lines--) {
++ guint8 packets;
++
+ /* packet count */
+- packets = *data++;
++ if (!gst_byte_reader_get_uint8 (reader, &packets))
++ goto error;
++ GST_LOG_OBJECT (flxdec, "have %d packets", packets);
+
+ while (packets--) {
+ /* skip count */
+- guchar skip = *data++;
+- dest += skip;
++ guint8 skip;
++ gint8 count;
++ if (!gst_byte_reader_get_uint8 (reader, &skip))
++ goto error;
++
++ /* skip bytes */
++ if (!gst_byte_writer_set_pos (writer,
++ gst_byte_writer_get_pos (writer) + skip))
++ goto error;
+
+ /* RLE count */
+- count = *data++;
++ if (!gst_byte_reader_get_int8 (reader, &count))
++ goto error;
++
++ if (count < 0) {
++ guint8 x;
+
+- if (count > 0x7f) {
+ /* literal run */
+- count = 0x100 - count;
++ count = ABS (count);
++ GST_LOG_OBJECT (flxdec, "have literal run of size %d at offset %d",
++ count, skip);
+
+ if (skip + count > flxdec->hdr.width) {
+ GST_ERROR_OBJECT (flxdec, "Invalid FLI packet detected. "
+@@ -393,11 +461,16 @@ flx_decode_delta_fli (GstFlxDec * flxdec, guchar * data, guchar * dest)
+ return FALSE;
+ }
+
+- x = *data++;
+- while (count--)
+- *dest++ = x;
+-
++ if (!gst_byte_reader_get_uint8 (reader, &x))
++ goto error;
++ if (!gst_byte_writer_fill (writer, x, count))
++ goto error;
+ } else {
++ const guint8 *data;
++
++ GST_LOG_OBJECT (flxdec, "have replicate run of size %d at offset %d",
++ count, skip);
++
+ if (skip + count > flxdec->hdr.width) {
+ GST_ERROR_OBJECT (flxdec, "Invalid FLI packet detected. "
+ "line too long.");
+@@ -405,45 +478,60 @@ flx_decode_delta_fli (GstFlxDec * flxdec, guchar * data, guchar * dest)
+ }
+
+ /* replicate run */
+- while (count--)
+- *dest++ = *data++;
++ if (!gst_byte_reader_get_data (reader, count, &data))
++ goto error;
++ if (!gst_byte_writer_put_data (writer, data, count))
++ goto error;
+ }
+ }
+- start_p += flxdec->hdr.width;
+- dest = start_p;
++ line_start_i += flxdec->hdr.width;
++ if (!gst_byte_writer_set_pos (writer, line_start_i))
++ goto error;
+ }
+
+ return TRUE;
++
++error:
++ GST_ERROR_OBJECT (flxdec, "Failed to decode FLI packet");
++ return FALSE;
+ }
+
+ static gboolean
+-flx_decode_delta_flc (GstFlxDec * flxdec, guchar * data, guchar * dest)
++flx_decode_delta_flc (GstFlxDec * flxdec, GstByteReader * reader,
++ GstByteWriter * writer)
+ {
+- gulong count, lines, start_l, opcode;
+- guchar *start_p;
++ guint16 lines, start_l;
+
+ g_return_val_if_fail (flxdec != NULL, FALSE);
+ g_return_val_if_fail (flxdec->delta_data != NULL, FALSE);
+
+ /* use last frame for delta */
+- memcpy (dest, flxdec->delta_data, flxdec->size);
++ if (!gst_byte_writer_put_data (writer, flxdec->delta_data, flxdec->size))
++ goto error;
++ if (!gst_byte_reader_get_uint16_le (reader, &lines))
++ goto error;
+
+- lines = (data[0] + (data[1] << 8));
+ if (lines > flxdec->hdr.height) {
+ GST_ERROR_OBJECT (flxdec, "Invalid FLC packet detected. too many lines.");
+ return FALSE;
+ }
+- data += 2;
+
+- start_p = dest;
+ start_l = lines;
+
+ while (lines) {
+- dest = start_p + (flxdec->hdr.width * (start_l - lines));
++ guint16 opcode;
++
++ if (!gst_byte_writer_set_pos (writer,
++ flxdec->hdr.width * (start_l - lines)))
++ goto error;
+
+ /* process opcode(s) */
+- while ((opcode = (data[0] + (data[1] << 8))) & 0xc000) {
+- data += 2;
++ while (TRUE) {
++ if (!gst_byte_reader_get_uint16_le (reader, &opcode))
++ goto error;
++ if ((opcode & 0xc000) == 0)
++ break;
++
+ if ((opcode & 0xc000) == 0xc000) {
+ /* line skip count */
+ gulong skip = (0x10000 - opcode);
+@@ -453,27 +541,44 @@ flx_decode_delta_flc (GstFlxDec * flxdec, guchar * data, guchar * dest)
+ return FALSE;
+ }
+ start_l += skip;
+- dest += flxdec->hdr.width * skip;
++ if (!gst_byte_writer_set_pos (writer,
++ gst_byte_writer_get_pos (writer) + flxdec->hdr.width * skip))
++ goto error;
+ } else {
+ /* last pixel */
+- dest += flxdec->hdr.width;
+- *dest++ = (opcode & 0xff);
++ if (!gst_byte_writer_set_pos (writer,
++ gst_byte_writer_get_pos (writer) + flxdec->hdr.width))
++ goto error;
++ if (!gst_byte_writer_put_uint8 (writer, opcode & 0xff))
++ goto error;
+ }
+ }
+- data += 2;
+
+ /* last opcode is the packet count */
++ GST_LOG_OBJECT (flxdec, "have %d packets", opcode);
+ while (opcode--) {
+ /* skip count */
+- guchar skip = *data++;
+- dest += skip;
++ guint8 skip;
++ gint8 count;
++
++ if (!gst_byte_reader_get_uint8 (reader, &skip))
++ goto error;
++ if (!gst_byte_writer_set_pos (writer,
++ gst_byte_writer_get_pos (writer) + skip))
++ goto error;
+
+ /* RLE count */
+- count = *data++;
++ if (!gst_byte_reader_get_int8 (reader, &count))
++ goto error;
++
++ if (count < 0) {
++ guint16 x;
+
+- if (count > 0x7f) {
+ /* replicate word run */
+- count = 0x100 - count;
++ count = ABS (count);
++
++ GST_LOG_OBJECT (flxdec, "have replicate run of size %d at offset %d",
++ count, skip);
+
+ if (skip + count > flxdec->hdr.width) {
+ GST_ERROR_OBJECT (flxdec, "Invalid FLC packet detected. "
+@@ -481,22 +586,31 @@ flx_decode_delta_flc (GstFlxDec * flxdec, guchar * data, guchar * dest)
+ return FALSE;
+ }
+
++ if (!gst_byte_reader_get_uint16_le (reader, &x))
++ goto error;
++
+ while (count--) {
+- *dest++ = data[0];
+- *dest++ = data[1];
++ if (!gst_byte_writer_put_uint16_le (writer, x)) {
++ goto error;
++ }
+ }
+- data += 2;
+ } else {
++ GST_LOG_OBJECT (flxdec, "have literal run of size %d at offset %d",
++ count, skip);
++
+ if (skip + count > flxdec->hdr.width) {
+ GST_ERROR_OBJECT (flxdec, "Invalid FLC packet detected. "
+ "line too long.");
+ return FALSE;
+ }
+
+- /* literal word run */
+ while (count--) {
+- *dest++ = *data++;
+- *dest++ = *data++;
++ guint16 x;
++
++ if (!gst_byte_reader_get_uint16_le (reader, &x))
++ goto error;
++ if (!gst_byte_writer_put_uint16_le (writer, x))
++ goto error;
+ }
+ }
+ }
+@@ -504,13 +618,91 @@ flx_decode_delta_flc (GstFlxDec * flxdec, guchar * data, guchar * dest)
+ }
+
+ return TRUE;
++
++error:
++ GST_ERROR_OBJECT (flxdec, "Failed to decode FLI packet");
++ return FALSE;
++}
++
++static gboolean
++_read_flx_header (GstFlxDec * flxdec, GstByteReader * reader, FlxHeader * flxh)
++{
++ memset (flxh, 0, sizeof (*flxh));
++
++ if (!gst_byte_reader_get_uint32_le (reader, &flxh->size))
++ goto error;
++ if (flxh->size < FlxHeaderSize) {
++ GST_ERROR_OBJECT (flxdec, "Invalid file size in the header");
++ return FALSE;
++ }
++
++ if (!gst_byte_reader_get_uint16_le (reader, &flxh->type))
++ goto error;
++ if (!gst_byte_reader_get_uint16_le (reader, &flxh->frames))
++ goto error;
++ if (!gst_byte_reader_get_uint16_le (reader, &flxh->width))
++ goto error;
++ if (!gst_byte_reader_get_uint16_le (reader, &flxh->height))
++ goto error;
++ if (!gst_byte_reader_get_uint16_le (reader, &flxh->depth))
++ goto error;
++ if (!gst_byte_reader_get_uint16_le (reader, &flxh->flags))
++ goto error;
++ if (!gst_byte_reader_get_uint32_le (reader, &flxh->speed))
++ goto error;
++ if (!gst_byte_reader_skip (reader, 2)) /* reserved */
++ goto error;
++ /* FLC */
++ if (!gst_byte_reader_get_uint32_le (reader, &flxh->created))
++ goto error;
++ if (!gst_byte_reader_get_uint32_le (reader, &flxh->creator))
++ goto error;
++ if (!gst_byte_reader_get_uint32_le (reader, &flxh->updated))
++ goto error;
++ if (!gst_byte_reader_get_uint32_le (reader, &flxh->updater))
++ goto error;
++ if (!gst_byte_reader_get_uint16_le (reader, &flxh->aspect_dx))
++ goto error;
++ if (!gst_byte_reader_get_uint16_le (reader, &flxh->aspect_dy))
++ goto error;
++ /* EGI */
++ if (!gst_byte_reader_get_uint16_le (reader, &flxh->ext_flags))
++ goto error;
++ if (!gst_byte_reader_get_uint16_le (reader, &flxh->keyframes))
++ goto error;
++ if (!gst_byte_reader_get_uint16_le (reader, &flxh->totalframes))
++ goto error;
++ if (!gst_byte_reader_get_uint32_le (reader, &flxh->req_memory))
++ goto error;
++ if (!gst_byte_reader_get_uint16_le (reader, &flxh->max_regions))
++ goto error;
++ if (!gst_byte_reader_get_uint16_le (reader, &flxh->transp_num))
++ goto error;
++ if (!gst_byte_reader_skip (reader, 24)) /* reserved */
++ goto error;
++ /* FLC */
++ if (!gst_byte_reader_get_uint32_le (reader, &flxh->oframe1))
++ goto error;
++ if (!gst_byte_reader_get_uint32_le (reader, &flxh->oframe2))
++ goto error;
++ if (!gst_byte_reader_skip (reader, 40)) /* reserved */
++ goto error;
++
++ return TRUE;
++
++error:
++ GST_ERROR_OBJECT (flxdec, "Error reading file header");
++ return FALSE;
+ }
+
+ static GstFlowReturn
+ gst_flxdec_chain (GstPad * pad, GstObject * parent, GstBuffer * buf)
+ {
++ GstByteReader reader;
++ GstBuffer *input;
++ GstMapInfo map_info;
+ GstCaps *caps;
+- guint avail;
++ guint available;
+ GstFlowReturn res = GST_FLOW_OK;
+
+ GstFlxDec *flxdec;
+@@ -521,31 +713,50 @@ gst_flxdec_chain (GstPad * pad, GstObject * parent, GstBuffer * buf)
+ g_return_val_if_fail (flxdec != NULL, GST_FLOW_ERROR);
+
+ gst_adapter_push (flxdec->adapter, buf);
+- avail = gst_adapter_available (flxdec->adapter);
++ available = gst_adapter_available (flxdec->adapter);
++ input = gst_adapter_get_buffer (flxdec->adapter, available);
++ if (!gst_buffer_map (input, &map_info, GST_MAP_READ)) {
++ GST_ELEMENT_ERROR (flxdec, STREAM, DECODE,
++ ("%s", "Failed to map buffer"), (NULL));
++ goto error;
++ }
++ gst_byte_reader_init (&reader, map_info.data, map_info.size);
+
+ if (flxdec->state == GST_FLXDEC_READ_HEADER) {
+- if (avail >= FlxHeaderSize) {
+- const guint8 *data = gst_adapter_map (flxdec->adapter, FlxHeaderSize);
++ if (available >= FlxHeaderSize) {
++ GstByteReader header;
+ GstCaps *templ;
+
+- memcpy ((gchar *) & flxdec->hdr, data, FlxHeaderSize);
+- FLX_HDR_FIX_ENDIANNESS (&(flxdec->hdr));
+- gst_adapter_unmap (flxdec->adapter);
++ if (!gst_byte_reader_get_sub_reader (&reader, &header, FlxHeaderSize)) {
++ GST_ELEMENT_ERROR (flxdec, STREAM, DECODE,
++ ("%s", "Could not read header"), (NULL));
++ goto unmap_input_error;
++ }
+ gst_adapter_flush (flxdec->adapter, FlxHeaderSize);
++ available -= FlxHeaderSize;
++
++ if (!_read_flx_header (flxdec, &header, &flxdec->hdr)) {
++ GST_ELEMENT_ERROR (flxdec, STREAM, DECODE,
++ ("%s", "Failed to parse header"), (NULL));
++ goto unmap_input_error;
++ }
+
+ flxh = &flxdec->hdr;
+
+ /* check header */
+ if (flxh->type != FLX_MAGICHDR_FLI &&
+- flxh->type != FLX_MAGICHDR_FLC && flxh->type != FLX_MAGICHDR_FLX)
+- goto wrong_type;
++ flxh->type != FLX_MAGICHDR_FLC && flxh->type != FLX_MAGICHDR_FLX) {
++ GST_ELEMENT_ERROR (flxdec, STREAM, WRONG_TYPE, (NULL),
++ ("not a flx file (type %x)", flxh->type));
++ goto unmap_input_error;
++ }
+
+- GST_LOG ("size : %d", flxh->size);
+- GST_LOG ("frames : %d", flxh->frames);
+- GST_LOG ("width : %d", flxh->width);
+- GST_LOG ("height : %d", flxh->height);
+- GST_LOG ("depth : %d", flxh->depth);
+- GST_LOG ("speed : %d", flxh->speed);
++ GST_INFO_OBJECT (flxdec, "size : %d", flxh->size);
++ GST_INFO_OBJECT (flxdec, "frames : %d", flxh->frames);
++ GST_INFO_OBJECT (flxdec, "width : %d", flxh->width);
++ GST_INFO_OBJECT (flxdec, "height : %d", flxh->height);
++ GST_INFO_OBJECT (flxdec, "depth : %d", flxh->depth);
++ GST_INFO_OBJECT (flxdec, "speed : %d", flxh->speed);
+
+ flxdec->next_time = 0;
+
+@@ -573,18 +784,32 @@ gst_flxdec_chain (GstPad * pad, GstObject * parent, GstBuffer * buf)
+ gst_pad_set_caps (flxdec->srcpad, caps);
+ gst_caps_unref (caps);
+
+- if (flxh->depth <= 8)
+- flxdec->converter =
+- flx_colorspace_converter_new (flxh->width, flxh->height);
++ /* zero means 8 */
++ if (flxh->depth == 0)
++ flxh->depth = 8;
++
++ if (flxh->depth != 8) {
++ GST_ELEMENT_ERROR (flxdec, STREAM, WRONG_TYPE,
++ ("%s", "Don't know how to decode non 8 bit depth streams"), (NULL));
++ goto unmap_input_error;
++ }
++
++ flxdec->converter =
++ flx_colorspace_converter_new (flxh->width, flxh->height);
+
+ if (flxh->type == FLX_MAGICHDR_FLC || flxh->type == FLX_MAGICHDR_FLX) {
+- GST_LOG ("(FLC) aspect_dx : %d", flxh->aspect_dx);
+- GST_LOG ("(FLC) aspect_dy : %d", flxh->aspect_dy);
+- GST_LOG ("(FLC) oframe1 : 0x%08x", flxh->oframe1);
+- GST_LOG ("(FLC) oframe2 : 0x%08x", flxh->oframe2);
++ GST_INFO_OBJECT (flxdec, "(FLC) aspect_dx : %d", flxh->aspect_dx);
++ GST_INFO_OBJECT (flxdec, "(FLC) aspect_dy : %d", flxh->aspect_dy);
++ GST_INFO_OBJECT (flxdec, "(FLC) oframe1 : 0x%08x", flxh->oframe1);
++ GST_INFO_OBJECT (flxdec, "(FLC) oframe2 : 0x%08x", flxh->oframe2);
+ }
+
+ flxdec->size = ((guint) flxh->width * (guint) flxh->height);
++ if (flxdec->size >= G_MAXSIZE / 4) {
++ GST_ELEMENT_ERROR (flxdec, STREAM, DECODE,
++ ("%s", "Cannot allocate required memory"), (NULL));
++ goto unmap_input_error;
++ }
+
+ /* create delta and output frame */
+ flxdec->frame_data = g_malloc (flxdec->size);
+@@ -596,55 +821,66 @@ gst_flxdec_chain (GstPad * pad, GstObject * parent, GstBuffer * buf)
+ GstBuffer *out;
+
+ /* while we have enough data in the adapter */
+- while (avail >= FlxFrameChunkSize && res == GST_FLOW_OK) {
+- FlxFrameChunk flxfh;
+- guchar *chunk;
+- const guint8 *data;
+- GstMapInfo map;
+-
+- chunk = NULL;
+- data = gst_adapter_map (flxdec->adapter, FlxFrameChunkSize);
+- memcpy (&flxfh, data, FlxFrameChunkSize);
+- FLX_FRAME_CHUNK_FIX_ENDIANNESS (&flxfh);
+- gst_adapter_unmap (flxdec->adapter);
+-
+- switch (flxfh.id) {
+- case FLX_FRAME_TYPE:
+- /* check if we have the complete frame */
+- if (avail < flxfh.size)
+- goto need_more_data;
+-
+- /* flush header */
+- gst_adapter_flush (flxdec->adapter, FlxFrameChunkSize);
+-
+- chunk = gst_adapter_take (flxdec->adapter,
+- flxfh.size - FlxFrameChunkSize);
+- FLX_FRAME_TYPE_FIX_ENDIANNESS ((FlxFrameType *) chunk);
+- if (((FlxFrameType *) chunk)->chunks == 0)
+- break;
++ while (available >= FlxFrameChunkSize && res == GST_FLOW_OK) {
++ guint32 size;
++ guint16 type;
+
+- /* create 32 bits output frame */
+-// res = gst_pad_alloc_buffer_and_set_caps (flxdec->srcpad,
+-// GST_BUFFER_OFFSET_NONE,
+-// flxdec->size * 4, GST_PAD_CAPS (flxdec->srcpad), &out);
+-// if (res != GST_FLOW_OK)
+-// break;
++ if (!gst_byte_reader_get_uint32_le (&reader, &size))
++ goto parse_error;
++ if (available < size)
++ goto need_more_data;
+
+- out = gst_buffer_new_and_alloc (flxdec->size * 4);
++ available -= size;
++ gst_adapter_flush (flxdec->adapter, size);
++
++ if (!gst_byte_reader_get_uint16_le (&reader, &type))
++ goto parse_error;
++
++ switch (type) {
++ case FLX_FRAME_TYPE:{
++ GstByteReader chunks;
++ GstByteWriter writer;
++ guint16 n_chunks;
++ GstMapInfo map;
++
++ GST_LOG_OBJECT (flxdec, "Have frame type 0x%02x of size %d", type,
++ size);
++
++ if (!gst_byte_reader_get_sub_reader (&reader, &chunks,
++ size - FlxFrameChunkSize))
++ goto parse_error;
++
++ if (!gst_byte_reader_get_uint16_le (&chunks, &n_chunks))
++ goto parse_error;
++ GST_LOG_OBJECT (flxdec, "Have %d chunks", n_chunks);
++
++ if (n_chunks == 0)
++ break;
++ if (!gst_byte_reader_skip (&chunks, 8)) /* reserved */
++ goto parse_error;
++
++ gst_byte_writer_init_with_data (&writer, flxdec->frame_data,
++ flxdec->size, TRUE);
+
+ /* decode chunks */
+- if (!flx_decode_chunks (flxdec,
+- ((FlxFrameType *) chunk)->chunks,
+- chunk + FlxFrameTypeSize, flxdec->frame_data)) {
++ if (!flx_decode_chunks (flxdec, n_chunks, &chunks, &writer)) {
+ GST_ELEMENT_ERROR (flxdec, STREAM, DECODE,
+ ("%s", "Could not decode chunk"), NULL);
+- return GST_FLOW_ERROR;
++ goto unmap_input_error;
+ }
++ gst_byte_writer_reset (&writer);
+
+ /* save copy of the current frame for possible delta. */
+ memcpy (flxdec->delta_data, flxdec->frame_data, flxdec->size);
+
+- gst_buffer_map (out, &map, GST_MAP_WRITE);
++ out = gst_buffer_new_and_alloc (flxdec->size * 4);
++ if (!gst_buffer_map (out, &map, GST_MAP_WRITE)) {
++ GST_ELEMENT_ERROR (flxdec, STREAM, DECODE,
++ ("%s", "Could not map output buffer"), NULL);
++ gst_buffer_unref (out);
++ goto unmap_input_error;
++ }
++
+ /* convert current frame. */
+ flx_colorspace_convert (flxdec->converter, flxdec->frame_data,
+ map.data);
+@@ -655,30 +891,32 @@ gst_flxdec_chain (GstPad * pad, GstObject * parent, GstBuffer * buf)
+
+ res = gst_pad_push (flxdec->srcpad, out);
+ break;
++ }
+ default:
+- /* check if we have the complete frame */
+- if (avail < flxfh.size)
+- goto need_more_data;
+-
+- gst_adapter_flush (flxdec->adapter, flxfh.size);
++ GST_DEBUG_OBJECT (flxdec, "Unknown frame type 0x%02x, skipping %d",
++ type, size);
++ if (!gst_byte_reader_skip (&reader, size - FlxFrameChunkSize))
++ goto parse_error;
+ break;
+ }
+-
+- g_free (chunk);
+-
+- avail = gst_adapter_available (flxdec->adapter);
+ }
+ }
++
++ gst_buffer_unmap (input, &map_info);
++ gst_buffer_unref (input);
++
+ need_more_data:
+ return res;
+
+ /* ERRORS */
+-wrong_type:
+- {
+- GST_ELEMENT_ERROR (flxdec, STREAM, WRONG_TYPE, (NULL),
+- ("not a flx file (type %x)", flxh->type));
+- return GST_FLOW_ERROR;
+- }
++parse_error:
++ GST_ELEMENT_ERROR (flxdec, STREAM, DECODE,
++ ("%s", "Failed to parse stream"), (NULL));
++unmap_input_error:
++ gst_buffer_unmap (input, &map_info);
++ gst_buffer_unref (input);
++error:
++ return GST_FLOW_ERROR;
+ }
+
+ static GstStateChangeReturn
+diff --git a/gst/flx/gstflxdec.h b/gst/flx/gstflxdec.h
+index 3f9a0aa..4fd8dfd 100644
+--- a/gst/flx/gstflxdec.h
++++ b/gst/flx/gstflxdec.h
+@@ -23,6 +23,8 @@
+ #include <gst/gst.h>
+
+ #include <gst/base/gstadapter.h>
++#include <gst/base/gstbytereader.h>
++#include <gst/base/gstbytewriter.h>
+ #include "flx_color.h"
+
+ G_BEGIN_DECLS
+@@ -45,7 +47,7 @@ struct _GstFlxDec {
+
+ guint8 *delta_data, *frame_data;
+ GstAdapter *adapter;
+- gulong size;
++ gsize size;
+ GstFlxDecState state;
+ gint64 frame_time;
+ gint64 next_time;
+--
+2.10.2
+
--
2.10.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 0/1] Gst-plugins-good security update
2016-11-25 7:11 [PATCH 0/1] Gst-plugins-good security update Leo Famulari
2016-11-25 7:11 ` [PATCH 1/1] gnu: gst-plugins-good: Fix CVE-2016-{9634,9635,9636} Leo Famulari
@ 2016-11-26 8:51 ` Marius Bakke
2016-11-26 17:54 ` Leo Famulari
2016-11-26 19:38 ` Leo Famulari
1 sibling, 2 replies; 6+ messages in thread
From: Marius Bakke @ 2016-11-26 8:51 UTC (permalink / raw)
To: Leo Famulari, guix-devel
[-- Attachment #1: Type: text/plain, Size: 1177 bytes --]
Leo Famulari <leo@famulari.name> writes:
> This patch should fix the bugs named here:
>
> http://seclists.org/oss-sec/2016/q4/517
>
> I copied Debian's approach, which is to take all the recent patches for
> the vulnerable component (the FLIC decoder).
>
> My understanding is that the first two patches fix the CVEs, the 3rd
> fixes an unrelated bug, and the 4th is a total rewrite of the component,
> because "code is terrible, it should be entirely re-written" [0].
>
> The CVE bug fixes are not split into discrete patches, so it doesn't
> work to make patches for each CVE ID, like we normally do.
>
> Is this approach (concatenating the patches) okay?
I prefer having them separately, so the upstream commit can be clearly
referenced in the patch header; and they can be reviewed and modified
independently.
In this instance it's okay, since I just checked out the 1.10 branch and
concatenated the four commits and ended up with the same patch :-)
That's not to say it should not be allowed. I think this approach is
fine for long patch series, but at only four patches it's not the best
precedent.
Anyway, thanks for taking care of this, and LGTM! Please push! :-)
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/1] Gst-plugins-good security update
2016-11-26 8:51 ` [PATCH 0/1] Gst-plugins-good security update Marius Bakke
@ 2016-11-26 17:54 ` Leo Famulari
2016-11-26 17:58 ` Marius Bakke
2016-11-26 19:38 ` Leo Famulari
1 sibling, 1 reply; 6+ messages in thread
From: Leo Famulari @ 2016-11-26 17:54 UTC (permalink / raw)
To: Marius Bakke; +Cc: guix-devel
[-- Attachment #1: Type: text/plain, Size: 995 bytes --]
On Sat, Nov 26, 2016 at 09:51:30AM +0100, Marius Bakke wrote:
> Leo Famulari <leo@famulari.name> writes:
> > The CVE bug fixes are not split into discrete patches, so it doesn't
> > work to make patches for each CVE ID, like we normally do.
> >
> > Is this approach (concatenating the patches) okay?
>
> I prefer having them separately, so the upstream commit can be clearly
> referenced in the patch header; and they can be reviewed and modified
> independently.
>
> In this instance it's okay, since I just checked out the 1.10 branch and
> concatenated the four commits and ended up with the same patch :-)
>
> That's not to say it should not be allowed. I think this approach is
> fine for long patch series, but at only four patches it's not the best
> precedent.
I wondered how to split the patches up here. I don't know how to name
the first two patches, since the CVE bug fixes are spread between them.
I'll break the 3 and 4th patch off into their own files.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/1] Gst-plugins-good security update
2016-11-26 17:54 ` Leo Famulari
@ 2016-11-26 17:58 ` Marius Bakke
0 siblings, 0 replies; 6+ messages in thread
From: Marius Bakke @ 2016-11-26 17:58 UTC (permalink / raw)
To: Leo Famulari; +Cc: guix-devel
[-- Attachment #1: Type: text/plain, Size: 279 bytes --]
Leo Famulari <leo@famulari.name> writes:
> I wondered how to split the patches up here. I don't know how to name
> the first two patches, since the CVE bug fixes are spread between them.
I tend to use or abbreviate the commit title, if there is no obvious
'fix-foo' available.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/1] Gst-plugins-good security update
2016-11-26 8:51 ` [PATCH 0/1] Gst-plugins-good security update Marius Bakke
2016-11-26 17:54 ` Leo Famulari
@ 2016-11-26 19:38 ` Leo Famulari
1 sibling, 0 replies; 6+ messages in thread
From: Leo Famulari @ 2016-11-26 19:38 UTC (permalink / raw)
To: Marius Bakke; +Cc: guix-devel
[-- Attachment #1: Type: text/plain, Size: 1478 bytes --]
On Sat, Nov 26, 2016 at 09:51:30AM +0100, Marius Bakke wrote:
> Leo Famulari <leo@famulari.name> writes:
>
> > This patch should fix the bugs named here:
> >
> > http://seclists.org/oss-sec/2016/q4/517
> >
> > I copied Debian's approach, which is to take all the recent patches for
> > the vulnerable component (the FLIC decoder).
> >
> > My understanding is that the first two patches fix the CVEs, the 3rd
> > fixes an unrelated bug, and the 4th is a total rewrite of the component,
> > because "code is terrible, it should be entirely re-written" [0].
> >
> > The CVE bug fixes are not split into discrete patches, so it doesn't
> > work to make patches for each CVE ID, like we normally do.
> >
> > Is this approach (concatenating the patches) okay?
>
> I prefer having them separately, so the upstream commit can be clearly
> referenced in the patch header; and they can be reviewed and modified
> independently.
>
> In this instance it's okay, since I just checked out the 1.10 branch and
> concatenated the four commits and ended up with the same patch :-)
>
> That's not to say it should not be allowed. I think this approach is
> fine for long patch series, but at only four patches it's not the best
> precedent.
>
> Anyway, thanks for taking care of this, and LGTM! Please push! :-)
I split them up and (hopefully) annotated them well enough that readers
can follow along.
Pushed as 9e46245b89e0f30397f69391a2219a29caa336a2
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-11-26 19:39 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-11-25 7:11 [PATCH 0/1] Gst-plugins-good security update Leo Famulari
2016-11-25 7:11 ` [PATCH 1/1] gnu: gst-plugins-good: Fix CVE-2016-{9634,9635,9636} Leo Famulari
2016-11-26 8:51 ` [PATCH 0/1] Gst-plugins-good security update Marius Bakke
2016-11-26 17:54 ` Leo Famulari
2016-11-26 17:58 ` Marius Bakke
2016-11-26 19:38 ` 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).