[FFmpeg-devel] [PATCH][RFC] nsv seeking

Jai Menon jmenon86
Mon Apr 20 16:33:40 CEST 2009


On Mon, Apr 20, 2009 at 6:24 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> On Sun, Apr 19, 2009 at 04:46:48PM +0530, Jai Menon wrote:
>> On 4/18/09, Fran?ois Revol <revol at free.fr> wrote:
> [...]
>> @@ -348,12 +349,22 @@ static int nsv_parse_NSVf_header(AVFormatContext *s, AVFormatParameters *ap)
>> ? ? ?PRINT(("NSV got infos; filepos %"PRId64"\n", url_ftell(pb)));
>>
>> ? ? ?if (table_entries_used > 0) {
>> + ? ? ? ?int i;
>> ? ? ? ? ?nsv->index_entries = table_entries_used;
>> ? ? ? ? ?if((unsigned)table_entries >= UINT_MAX / sizeof(uint32_t))
>> ? ? ? ? ? ? ?return -1;
>> - ? ? ? ?nsv->nsvs_file_offset = av_malloc(table_entries * sizeof(uint32_t));
>> -#warning "FIXME: Byteswap buffer as needed"
>> - ? ? ? ?get_buffer(pb, (unsigned char *)nsv->nsvs_file_offset, table_entries * sizeof(uint32_t));
>> + ? ? ? ?nsv->nsvs_file_offset = av_malloc((unsigned)table_entries_used * sizeof(uint32_t));
>> +
>> + ? ? ? ?for(i=0;i<table_entries_used;i++)
>> + ? ? ? ? ? ?nsv->nsvs_file_offset[i] = get_le32(pb) + size;
>
> exploitable

I can't think of any feasible attack vector here. Could you be a little verbose.

>> +
>> + ? ? ? ?if(table_entries > table_entries_used &&
>> + ? ? ? ? ? get_le32(pb) == MKTAG('T','O','C','2')) {
>> + ? ? ? ? ? ?nsv->nsvs_timestamps = av_malloc((unsigned)table_entries_used*sizeof(uint32_t));
>> + ? ? ? ? ? ?for(i=0;i<table_entries_used;i++) {
>> + ? ? ? ? ? ? ? ?nsv->nsvs_timestamps[i] = get_le32(pb);
>> + ? ? ? ? ? ?}
>> + ? ? ? ?}
>> ? ? ?}
>>
>> ? ? ?PRINT(("NSV got index; filepos %"PRId64"\n", url_ftell(pb)));
>
> the variable names table_entries and table_entries_used should be
> changed

Hmm...there is a lot of possibility in that file for cleanup. Could
that be tackled separately?

> [...]
>> @@ -700,9 +726,10 @@ static int nsv_read_close(AVFormatContext *s)
>> ?/* ? ? int i; */
>> ? ? ?NSVContext *nsv = s->priv_data;
>>
>> - ? ?if (nsv->index_entries)
>> + ? ?if (nsv->index_entries) {
>
> superflous i suspect

Yeah, that should be an av_freep, but as I said, lot of possibility for cleanup.


-- 
Regards,

Jai



More information about the ffmpeg-devel mailing list