After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 333488 - Allow for palette < 256 colours in AVI files
Allow for palette < 256 colours in AVI files
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other All
: Normal normal
: 0.10.4
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2006-03-05 14:22 UTC by Fabrizio Gennari
Modified: 2006-03-14 15:13 UTC
See Also:
GNOME target: ---
GNOME version: Unversioned Enhancement


Attachments
Patch for allowing palettes with less tahn 256 colours in AVI files (1.11 KB, patch)
2006-03-05 14:24 UTC, Fabrizio Gennari
committed Details | Review
Small modification (1.54 KB, patch)
2006-03-09 20:14 UTC, Fabrizio Gennari
committed Details | Review
Smaller modification (703 bytes, patch)
2006-03-13 22:48 UTC, Fabrizio Gennari
committed Details | Review

Description Fabrizio Gennari 2006-03-05 14:22:48 UTC
Some AVI files have a palette of less than 256 colours. Currently, GStreamer
ignores the palette if it has less than 256 colours, but it is legal to have a
smaller palette
Comment 1 Fabrizio Gennari 2006-03-05 14:24:19 UTC
Created attachment 60692 [details] [review]
Patch for allowing palettes with less tahn 256 colours in AVI files

With this patch, the number of colours is checked, and the palette is added even if it has less than 256 colours
Comment 2 Tim-Philipp Müller 2006-03-08 09:37:39 UTC
Committed with minor modifications, thanks!

 2006-03-08  Tim-Philipp Müller  <tim at centricular dot net>

       * gst-libs/gst/riff/riff-media.c: (gst_riff_create_video_caps):
         Allow palettes with less than 256 colours in AVI files
         (#333488, patch by: Fabrizio Gennari).

Comment 3 Fabrizio Gennari 2006-03-09 20:14:19 UTC
Created attachment 61000 [details] [review]
Small modification

May I suggest a small improvement?

In the current code, GST_BUFFER_SIZE(palette) is never checked, in particular it is never checked if it is > GST_BUFFER_SIZE(copy). Well, I never found a practical case, but theoretically it could happen.

This patch should prevent overflows.
Comment 4 Tim-Philipp Müller 2006-03-10 08:54:55 UTC
Very right, I messed that up. Sorry.

Should probably go into the pending release, as it is security relevant to some extent. Thomas, the addition is only a

+    if (GST_BUFFER_SIZE (palette) >= num_colors * 4) {

+    }

can this go in?
Comment 5 Tim-Philipp Müller 2006-03-10 11:09:26 UTC
Committed, thanks for catching this.

2006-03-10  Tim-Philipp Müller  <tim at centricular dot net>

       * gst-libs/gst/riff/riff-media.c: (gst_riff_create_video_caps):
         Make sure we don't read beyond the palette buffer in case of
         broken or manipulated files (#333488, patch by: Fabrizio
         Gennari)

Comment 6 Fabrizio Gennari 2006-03-13 22:48:14 UTC
Created attachment 61209 [details] [review]
Smaller modification

A one-liner to ensure GST_BUFFER_DATA (copy) >= GST_BUFFER_DATA (palette) always (not that I ever found such a case, but you never know...).

This is the last time I'm picky, promise!
Comment 7 Tim-Philipp Müller 2006-03-14 15:13:39 UTC
Nothing better than a good dose of paranoia and due caution ;)

Committed:

2006-03-14  Tim-Philipp Müller  <tim at centricular dot net>

        Patch by: Fabrizio Gennari  <fabrizio dot ge at tiscali dot it>

        * gst-libs/gst/riff/riff-media.c: (gst_riff_create_video_caps):
          Make sure the buffer we copy into is really always big
          enough, this time for real (#333488).