Re: [FFmpeg-devel] [PATCH] oggparsedaala: reject too large gpshift

[Andreas Cadhalpun]

On 29.12.2015 22:27, Rostislav Pehlivanov wrote:
> oggparsetheora has the same bit of code to read the gpshift, so it would
> probably be a good idea to add it to this patch as well.

No, oggparsetheora only reads 5 bits for gpshift.
The only thing from this patch that also applies there is the (theoretical)
issue of 1<<31 not being defined for int32_t.

On 29.12.2015 22:32, Hendrik Leppkes wrote:
> 1U << hdr->gpshift?

Sure. Updated patch attached.

Best regards,
Andreas

>From 4380123388f38eb9bbd11db34b0ac82a9ec18d5a Mon Sep 17 00:00:00 2001
From: Andreas Cadhalpun <andreas.cadhal...@googlemail.com>
Date: Tue, 29 Dec 2015 18:32:01 +0100
Subject: [PATCH] oggparsedaala: reject too large gpshift

Also use a unsigned constant for the shift calculation, as 1 << 31 is
undefined for int32_t. This is also fixed oggparsetheora.

This fixes ubsan runtime error: shift exponent is too large for
32-bit type 'int'

Signed-off-by: Andreas Cadhalpun <andreas.cadhal...@googlemail.com>
---
 libavformat/oggparsedaala.c  | 7 ++++++-
 libavformat/oggparsetheora.c | 2 +-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/libavformat/oggparsedaala.c b/libavformat/oggparsedaala.c
index 24567f9..3651ca1 100644
--- a/libavformat/oggparsedaala.c
+++ b/libavformat/oggparsedaala.c
@@ -123,7 +123,12 @@ static int daala_header(AVFormatContext *s, int idx)
 
         hdr->frame_duration = bytestream2_get_ne32(&gb);
         hdr->gpshift = bytestream2_get_byte(&gb);
-        hdr->gpmask  = (1 << hdr->gpshift) - 1;
+        if (hdr->gpshift >= 32) {
+            av_log(s, AV_LOG_ERROR, "Too large gpshift %d (>= 32).\n",
+                   hdr->gpshift);
+            return AVERROR_INVALIDDATA;
+        }
+        hdr->gpmask  = (1U << hdr->gpshift) - 1;
 
         hdr->format.depth  = 8 + 2*(bytestream2_get_byte(&gb)-1);
 
diff --git a/libavformat/oggparsetheora.c b/libavformat/oggparsetheora.c
index 6e6a362..5f057c3 100644
--- a/libavformat/oggparsetheora.c
+++ b/libavformat/oggparsetheora.c
@@ -108,7 +108,7 @@ static int theora_header(AVFormatContext *s, int idx)
             skip_bits(&gb, 2);
 
         thp->gpshift = get_bits(&gb, 5);
-        thp->gpmask  = (1 << thp->gpshift) - 1;
+        thp->gpmask  = (1U << thp->gpshift) - 1;
 
         st->codec->codec_type = AVMEDIA_TYPE_VIDEO;
         st->codec->codec_id   = AV_CODEC_ID_THEORA;
-- 
2.6.4


_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel