On 04/27/2013 10:14 AM, Anton Khirnov wrote:
>
> On Fri, 19 Apr 2013 23:37:42 +0200, Luca Barbato <[email protected]> wrote:
>> Prevent read after buffer boundary on corrupted tag.
>>
>> Reported-by: Mateusz "j00ru" Jurczyk and Gynvael Coldwind
>> CC: [email protected]
>> ---
>> libavformat/omadec.c | 19 +++++++++++++------
>> 1 file changed, 13 insertions(+), 6 deletions(-)
>>
>> diff --git a/libavformat/omadec.c b/libavformat/omadec.c
>> index b3765f7..0a8bf9f 100644
>> --- a/libavformat/omadec.c
>> +++ b/libavformat/omadec.c
>> @@ -142,31 +142,38 @@ static int rprobe(AVFormatContext *s, uint8_t
>> *enc_header, const uint8_t *r_val)
>> return memcmp(&enc_header[pos], oc->sm_val, 8) ? -1 : 0;
>> }
>>
>> -static int nprobe(AVFormatContext *s, uint8_t *enc_header, int size,
>> +#define PROBE_SIZE 4 + 4 + 4 + 4
>> +
>> +static int nprobe(AVFormatContext *s, uint8_t *enc_header, unsigned size,
>
> How is this related?
>
>> const uint8_t *n_val)
>> {
>> OMAContext *oc = s->priv_data;
>> - uint32_t pos, taglen, datalen;
>> + uint64_t pos;
>> + uint32_t taglen, datalen;
>> struct AVDES av_des;
>>
>> - if (!enc_header || !n_val)
>> + if (!enc_header || !n_val ||
>> + size < OMA_ENC_HEADER_SIZE + oc->k_size + PROBE_SIZE)
>> return -1;
>>
>> pos = OMA_ENC_HEADER_SIZE + oc->k_size;
>> if (!memcmp(&enc_header[pos], "EKB ", 4))
>> pos += 32;
>>
>> + if (size < pos)
>> + return -1;
>
> Doesn't look sufficient.
> If you incremented by 32 above, then you need an additional check for that.
Right.
>> +
>> if (AV_RB32(&enc_header[pos]) != oc->rid)
>> av_log(s, AV_LOG_DEBUG, "Mismatching RID\n");
>>
>> taglen = AV_RB32(&enc_header[pos + 32]);
>> datalen = AV_RB32(&enc_header[pos + 36]) >> 4;
>
> Those aren't checked at all.
>
>>
>> - if (taglen + (((uint64_t)datalen) << 4) + 44 > size)
>> - return -1;
>> -
>> pos += 44 + taglen;
>>
>> + if (datalen << 4 > size - pos)
>> + return -1;
>> +
>> av_des_init(&av_des, n_val, 192, 1);
>> while (datalen-- > 0) {
>> av_des_crypt(&av_des, oc->r_val, &enc_header[pos], 2, NULL, 1);
>
> And it seems some checks are also needed for rprobe that's called lower.
Right, again, thanks for the review =)
lu
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
