[MPlayer-cvslog] CVS: main/libmpdemux aviheader.c,1.72,1.73

[Ivan Kalvachev]

2006/3/29, Alban Bedel CVS <syncmail at mplayerhq.hu>:
> CVS change done by Alban Bedel CVS
>
> Update of /cvsroot/mplayer/main/libmpdemux
> In directory mail:/var2/tmp/cvs-serv13130/libmpdemux
>
> Modified Files:
>         aviheader.c
> Log Message:
> 100L to who wrote this. Also fix broken headers while at it.
>
>
> Index: aviheader.c
> ===================================================================
> RCS file: /cvsroot/mplayer/main/libmpdemux/aviheader.c,v
> retrieving revision 1.72
> retrieving revision 1.73
> diff -u -r1.72 -r1.73
> --- aviheader.c 27 Mar 2006 17:25:41 -0000      1.72
> +++ aviheader.c 29 Mar 2006 19:31:31 -0000      1.73
> @@ -225,7 +225,12 @@
>
>        print_avisuperindex_chunk(s,MSGL_V);
>
> -      msize = sizeof (uint32_t) * s->wLongsPerEntry * s->nEntriesInUse;
> +      // Check and fix this useless crap
> +      if(s->wLongsPerEntry != sizeof (avisuperindex_entry)/4) {
> +          mp_msg (MSGT_HEADER, MSGL_WARN, "Broken super index chunk size: %u\n",s->wLongsPerEntry);
> +          s->wLongsPerEntry = sizeof(avisuperindex_entry)/4;
> +      }
> +      msize = sizeof (avisuperindex_entry) * s->nEntriesInUse;
>        s->aIndex = malloc(msize);
>        memset (s->aIndex, 0, msize);
>        s->stdidx = malloc (s->nEntriesInUse * sizeof (avistdindex_chunk));

It is broken and it doesn't fix the dangerous overflow.

The wLongsPerEntry could vary and even the OpenDML specification
doesn't give range for it.

Your "fix" does not prevent overflow of msize.

Indeed the parsing is broken (we must advance with the
wLongsPerEntry*sizeof(long) on each iteration).


2006/3/30, Reimar Döffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de>:
> Hi,
> On Wed, Mar 29, 2006 at 09:31:34PM +0200, Alban Bedel CVS wrote:
> > +      msize = sizeof (avisuperindex_entry) * s->nEntriesInUse;
>
> We still need a sanity check for nEntriesInUse somewhere...
>

Indeed. For sanity check, we must use the size of the index chunk (i
will assume this is size2), so:

(size2-sizeof(indx_table)) / (s->wLongsPerEntry*sizeof(dword)) >= nEntriesInUse;

and if sizeof(avisuperindex_entry) < wLongsPerEntry*4 then msize will
never overflow.