On 09.11.2016 21:55, Michael Niedermayer wrote:
> On Wed, Nov 09, 2016 at 09:05:17PM +0100, Andreas Cadhalpun wrote:
>> On 09.11.2016 11:10, Michael Niedermayer wrote:
>>> On Wed, Nov 09, 2016 at 01:11:29AM +0100, Andreas Cadhalpun wrote:
>>>> Otherwise put_bits can be called with a value that doesn't fit in the
>>>> sample_len, causing an assertion failure.
>>>> ---
>>>> libavcodec/pnmdec.c | 4 ++++
>>>> 1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/libavcodec/pnmdec.c b/libavcodec/pnmdec.c
>>>> index ca97cc3..0381ea6 100644
>>>> --- a/libavcodec/pnmdec.c
>>>> +++ b/libavcodec/pnmdec.c
>>>> @@ -145,6 +145,10 @@ static int pnm_decode_frame(AVCodecContext *avctx,
>>>> void *data,
>>>> /* read a sequence of digits */
>>>> do {
>>>> v = 10*v + c;
>>>> + if (v > s->maxval) {
>>>> + av_log(avctx, AV_LOG_ERROR, "value %d
>>>> larger than maxval %d\n", v, s->maxval);
>>>> + return AVERROR_INVALIDDATA;
>>>> + }
>>>
>>> indention is a bit noisy
>>
>> Fixed.
>>
>>> i think it can overflow if maxval is large,
>>
>> I've added an explicit check for v < 0, which should catch that.
>>
>>> it would be faster to check outside the loop
>>
>> However, such a check could pass if v overflowed so much that it's
>> in the valid range again, so I'd rather not do that.
>>
>> Updated patch is attached.
>>
>> Best regards,
>> Andreas
>>
>
>> pnmdec.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>> 9b84227c054610b73977e3acde32734a47e46c0c
>> 0001-pnmdec-make-sure-v-is-capped-by-maxval.patch
>> From 7e9dcbde04ad95fc93ac2f0e91d734c8187c8d2b Mon Sep 17 00:00:00 2001
>> From: Andreas Cadhalpun <andreas.cadhal...@googlemail.com>
>> Date: Wed, 9 Nov 2016 01:09:35 +0100
>> Subject: [PATCH] pnmdec: make sure v is capped by maxval
>>
>> Otherwise put_bits can be called with a value that doesn't fit in the
>> sample_len, causing an assertion failure.
>> ---
>> libavcodec/pnmdec.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/libavcodec/pnmdec.c b/libavcodec/pnmdec.c
>> index ca97cc3..8961310 100644
>> --- a/libavcodec/pnmdec.c
>> +++ b/libavcodec/pnmdec.c
>> @@ -145,6 +145,10 @@ static int pnm_decode_frame(AVCodecContext *avctx, void
>> *data,
>> /* read a sequence of digits */
>> do {
>> v = 10*v + c;
>> + if (v < 0 || v > s->maxval) {
>
> v < 0 implies v is signed
> if 10*v overflows you have undefined behavior
Alright, let's use a more complex check. See attached patch.
Best regards,
Andreas
>From f315a3cfe35377a2638dc2294200a288408dc784 Mon Sep 17 00:00:00 2001
From: Andreas Cadhalpun <andreas.cadhal...@googlemail.com>
Date: Wed, 9 Nov 2016 01:09:35 +0100
Subject: [PATCH] pnmdec: make sure v is capped by maxval
Otherwise put_bits can be called with a value that doesn't fit in the
sample_len, causing an assertion failure.
---
libavcodec/pnmdec.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/libavcodec/pnmdec.c b/libavcodec/pnmdec.c
index ca97cc3..0f6a895 100644
--- a/libavcodec/pnmdec.c
+++ b/libavcodec/pnmdec.c
@@ -144,6 +144,10 @@ static int pnm_decode_frame(AVCodecContext *avctx, void *data,
} else {
/* read a sequence of digits */
do {
+ if (v > (INT_MAX - c) / 10 || 10 * v + c > s->maxval) {
+ av_log(avctx, AV_LOG_ERROR, "value 10 * %d + %d larger than maxval %d\n", v, c, s->maxval);
+ return AVERROR_INVALIDDATA;
+ }
v = 10*v + c;
c = (*s->bytestream++) - '0';
} while (c <= 9);
--
2.10.2
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
