[07/11] fraps: Make the input buffer size checks more strict

Message ID 1379599756-27062-7-git-send-email-martin@martin.st
State Committed
Headers show

Commit Message

Martin Storsjö Sept. 19, 2013, 2:09 p.m.
Reported-by: Mateusz "j00ru" Jurczyk and Gynvael Coldwind
CC: libav-stable@libav.org
---
 libavcodec/fraps.c |   30 +++++++++++++++++++-----------
 1 file changed, 19 insertions(+), 11 deletions(-)

Comments

Kostya Shishkov Sept. 19, 2013, 2:23 p.m. | #1
On Thu, Sep 19, 2013 at 05:09:12PM +0300, Martin Storsjö wrote:
> Reported-by: Mateusz "j00ru" Jurczyk and Gynvael Coldwind
> CC: libav-stable@libav.org
> ---
>  libavcodec/fraps.c |   30 +++++++++++++++++++-----------
>  1 file changed, 19 insertions(+), 11 deletions(-)
> 
> diff --git a/libavcodec/fraps.c b/libavcodec/fraps.c
> index 66ecdef..b969031 100644
> --- a/libavcodec/fraps.c
> +++ b/libavcodec/fraps.c
> @@ -145,10 +145,17 @@ static int decode_frame(AVCodecContext *avctx,
>      uint32_t offs[4];
>      int i, j, ret, is_chroma, planes;
>      enum AVPixelFormat pix_fmt;
> +    int prev_pic_bit, expected_size;
> +
> +    if (buf_size < 4) {
> +        av_log(avctx, AV_LOG_ERROR, "Too short packet\n");

nit: Packet is too short


at quick glance looks OK

Patch

diff --git a/libavcodec/fraps.c b/libavcodec/fraps.c
index 66ecdef..b969031 100644
--- a/libavcodec/fraps.c
+++ b/libavcodec/fraps.c
@@ -145,10 +145,17 @@  static int decode_frame(AVCodecContext *avctx,
     uint32_t offs[4];
     int i, j, ret, is_chroma, planes;
     enum AVPixelFormat pix_fmt;
+    int prev_pic_bit, expected_size;
+
+    if (buf_size < 4) {
+        av_log(avctx, AV_LOG_ERROR, "Too short packet\n");
+        return AVERROR_INVALIDDATA;
+    }
 
     header      = AV_RL32(buf);
     version     = header & 0xff;
     header_size = (header & (1<<30))? 8 : 4; /* bit 30 means pad to 8 bytes */
+    prev_pic_bit = header & (1U << 31); /* bit 31 means same as previous pic */
 
     if (version > 5) {
         av_log(avctx, AV_LOG_ERROR,
@@ -167,16 +174,18 @@  static int decode_frame(AVCodecContext *avctx,
     }
     avctx->pix_fmt = pix_fmt;
 
+    expected_size = header_size;
+
     switch (version) {
     case 0:
     default:
         /* Fraps v0 is a reordered YUV420 */
-        if ((buf_size != avctx->width * avctx->height * 3 / 2 + header_size) &&
-            (buf_size != header_size)) {
+        if (!prev_pic_bit)
+            expected_size += avctx->width * avctx->height * 3 / 2;
+        if (buf_size != expected_size) {
             av_log(avctx, AV_LOG_ERROR,
                    "Invalid frame length %d (should be %d)\n",
-                   buf_size,
-                   avctx->width * avctx->height * 3 / 2 + header_size);
+                   buf_size, expected_size);
             return AVERROR_INVALIDDATA;
         }
 
@@ -190,8 +199,7 @@  static int decode_frame(AVCodecContext *avctx,
             av_log(avctx, AV_LOG_ERROR, "reget_buffer() failed\n");
             return ret;
         }
-        /* bit 31 means same as previous pic */
-        f->pict_type = (header & (1U << 31)) ? AV_PICTURE_TYPE_P : AV_PICTURE_TYPE_I;
+        f->pict_type = prev_pic_bit ? AV_PICTURE_TYPE_P : AV_PICTURE_TYPE_I;
         f->key_frame = f->pict_type == AV_PICTURE_TYPE_I;
 
         if (f->pict_type == AV_PICTURE_TYPE_I) {
@@ -215,11 +223,12 @@  static int decode_frame(AVCodecContext *avctx,
 
     case 1:
         /* Fraps v1 is an upside-down BGR24 */
-        if ((buf_size != avctx->width * avctx->height * 3 + header_size) &&
-            (buf_size != header_size) ) {
+        if (!prev_pic_bit)
+            expected_size += avctx->width * avctx->height * 3;
+        if (buf_size != expected_size) {
             av_log(avctx, AV_LOG_ERROR,
                    "Invalid frame length %d (should be %d)\n",
-                   buf_size, avctx->width * avctx->height * 3 + header_size);
+                   buf_size, expected_size);
             return AVERROR_INVALIDDATA;
         }
 
@@ -227,8 +236,7 @@  static int decode_frame(AVCodecContext *avctx,
             av_log(avctx, AV_LOG_ERROR, "reget_buffer() failed\n");
             return ret;
         }
-        /* bit 31 means same as previous pic */
-        f->pict_type = (header & (1U<<31))? AV_PICTURE_TYPE_P : AV_PICTURE_TYPE_I;
+        f->pict_type = prev_pic_bit ? AV_PICTURE_TYPE_P : AV_PICTURE_TYPE_I;
         f->key_frame = f->pict_type == AV_PICTURE_TYPE_I;
 
         if (f->pict_type == AV_PICTURE_TYPE_I) {