[05/18] rv10: Validate the dimensions set from the container
diff mbox

Message ID 1379358389-64839-5-git-send-email-martin@martin.st
State Superseded
Headers show

Commit Message

Martin Storsjö Sept. 16, 2013, 7:06 p.m. 
Reported-by: Mateusz "j00ru" Jurczyk and Gynvael Coldwind
CC: libav-stable@libav.org
---
 libavcodec/rv10.c |    4 ++++
 1 file changed, 4 insertions(+)

Comments

Josh Allmann Sept. 16, 2013, 9:28 p.m. | #1 
On 16 September 2013 12:06, Martin Storsjö <martin@martin.st> wrote:
> Reported-by: Mateusz "j00ru" Jurczyk and Gynvael Coldwind
> CC: libav-stable@libav.org
> ---
>  libavcodec/rv10.c |    4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/libavcodec/rv10.c b/libavcodec/rv10.c
> index 0d3b648..a73ebfe 100644
> --- a/libavcodec/rv10.c
> +++ b/libavcodec/rv10.c
> @@ -432,6 +432,10 @@ static av_cold int rv10_decode_init(AVCodecContext *avctx)
>          av_log(avctx, AV_LOG_ERROR, "Extradata is too small.\n");
>          return AVERROR_INVALIDDATA;
>      }
> +    if (avctx->coded_width <= 0 || avctx->coded_height <= 0) {
> +        av_log(avctx, AV_LOG_ERROR, "Invalid picture dimensions\n");
> +        return AVERROR_INVALIDDATA;
> +    }
>

Looks OK, is there a maximum dimension?
Martin Storsjö Sept. 17, 2013, 8:24 a.m. | #2 
On Mon, 16 Sep 2013, Josh Allmann wrote:

> On 16 September 2013 12:06, Martin Storsjö <martin@martin.st> wrote:
>> Reported-by: Mateusz "j00ru" Jurczyk and Gynvael Coldwind
>> CC: libav-stable@libav.org
>> ---
>>  libavcodec/rv10.c |    4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/libavcodec/rv10.c b/libavcodec/rv10.c
>> index 0d3b648..a73ebfe 100644
>> --- a/libavcodec/rv10.c
>> +++ b/libavcodec/rv10.c
>> @@ -432,6 +432,10 @@ static av_cold int rv10_decode_init(AVCodecContext *avctx)
>>          av_log(avctx, AV_LOG_ERROR, "Extradata is too small.\n");
>>          return AVERROR_INVALIDDATA;
>>      }
>> +    if (avctx->coded_width <= 0 || avctx->coded_height <= 0) {
>> +        av_log(avctx, AV_LOG_ERROR, "Invalid picture dimensions\n");
>> +        return AVERROR_INVALIDDATA;
>> +    }
>>
>
> Looks OK, is there a maximum dimension?

No idea, but that normally should be caught by e.g. allocations of frames 
failing, unless there's some other reason to limit it (e.g. avoiding 
arithmetic overflow somewhere or so).

// Martin
Kostya Shishkov Sept. 17, 2013, 8:30 a.m. | #3 
On Tue, Sep 17, 2013 at 11:24:57AM +0300, Martin Storsjö wrote:
> On Mon, 16 Sep 2013, Josh Allmann wrote:
> 
> >On 16 September 2013 12:06, Martin Storsjö <martin@martin.st> wrote:
> >>Reported-by: Mateusz "j00ru" Jurczyk and Gynvael Coldwind
> >>CC: libav-stable@libav.org
> >>---
> >> libavcodec/rv10.c |    4 ++++
> >> 1 file changed, 4 insertions(+)
> >>
> >>diff --git a/libavcodec/rv10.c b/libavcodec/rv10.c
> >>index 0d3b648..a73ebfe 100644
> >>--- a/libavcodec/rv10.c
> >>+++ b/libavcodec/rv10.c
> >>@@ -432,6 +432,10 @@ static av_cold int rv10_decode_init(AVCodecContext *avctx)
> >>         av_log(avctx, AV_LOG_ERROR, "Extradata is too small.\n");
> >>         return AVERROR_INVALIDDATA;
> >>     }
> >>+    if (avctx->coded_width <= 0 || avctx->coded_height <= 0) {
> >>+        av_log(avctx, AV_LOG_ERROR, "Invalid picture dimensions\n");
> >>+        return AVERROR_INVALIDDATA;
> >>+    }
> >>
> >
> >Looks OK, is there a maximum dimension?
> 
> No idea, but that normally should be caught by e.g. allocations of
> frames failing, unless there's some other reason to limit it (e.g.
> avoiding arithmetic overflow somewhere or so).

we have av_image_check_size()

Patch
diff mbox 

diff --git a/libavcodec/rv10.c b/libavcodec/rv10.c
index 0d3b648..a73ebfe 100644
--- a/libavcodec/rv10.c
+++ b/libavcodec/rv10.c
@@ -432,6 +432,10 @@  static av_cold int rv10_decode_init(AVCodecContext *avctx)
         av_log(avctx, AV_LOG_ERROR, "Extradata is too small.\n");
         return AVERROR_INVALIDDATA;
     }
+    if (avctx->coded_width <= 0 || avctx->coded_height <= 0) {
+        av_log(avctx, AV_LOG_ERROR, "Invalid picture dimensions\n");
+        return AVERROR_INVALIDDATA;
+    }
 
     ff_MPV_decode_defaults(s);