[DVDnav-discuss] [PATCH] libdvdread: Fix crash when PTT is too short

[Dominik 'Rathann' Mierzejewski]

Hi John,

On Saturday, 09 July 2011 at 01:05, John Stebbins wrote:
> On 07/08/2011 03:41 PM, John Stebbins wrote:
> >Region 1 True Grit is an example of this.  The PTT that is allocated and 
> >read is smaller than what gets referenced. The data is byte-swapped in 
> >place which results in writes to memory locations outside the allocated 
> >region.
> >
> Remove an unnecessary and incorrect part of the patch.  There's some code 
> may lead to future invalid references, but I read the code wrong and 
> applied an incorrect fix.  Since I don't have a disc that actually provokes 
> this case, it is probably best to just leave it alone till such a disc 
> surfaces.
> 

> Index: ifo_read.c
> ===================================================================
> --- libdvdread.orig/src/ifo_read.c	2009-10-29 09:11:32.066743831 -0700
> +++ libdvdread/src/ifo_read.c	2009-11-13 10:27:49.293174360 -0800
> @@ -1138,6 +1127,14 @@
>      return 0;
>    }
>  
> +  if(vts_ptt_srpt->nr_of_srpts * sizeof(uint32_t) > info_length) {

I assume that sizeof(uint32_t) comes from a few lines above:

data = (uint32_t *)malloc(info_length);

Why not use sizeof(*data)?

> +    fprintf(stderr, "libdvdread: PTT search table too small.\n");
> +    free(vts_ptt_srpt);
> +    free(data);
> +    ifofile->vts_ptt_srpt = 0;
> +    return 0;

This is duplicated code, maybe add a goto and move this (and similar 
snippets) to the end?

Regards,
Dominik

-- 
Fedora http://fedoraproject.org/wiki/User:Rathann
RPMFusion http://rpmfusion.org | MPlayer http://mplayerhq.hu
"Faith manages."
        -- Delenn to Lennier in Babylon 5:"Confessions and Lamentations"