[FFmpeg-devel] [PATCH] dvbsubdec: check against buffer overreads

[Reimar Döffinger]

On Thu, Feb 10, 2011 at 08:30:24PM +0000, M?ns Rullg?rd wrote:
> Reimar D?ffinger <Reimar.Doeffinger at gmx.de> writes:
> 
> > On Thu, Feb 10, 2011 at 08:08:50PM +0000, M?ns Rullg?rd wrote:
> >> Reimar D?ffinger <Reimar.Doeffinger at gmx.de> writes:
> >> 
> >> >> -    while (p < p_end && *p == 0x0f) {
> >> >> +    while (p_end - p >= 6 && *p == 0x0f) {
> >> >
> >> > Just to be pedantic: while this is nicer, thanks to required padding
> >> > your original version could not overflow, so wasn't actually wrong.
> >> 
> >> Why not?  It is certainly possible to place a buffer less than 6 bytes
> >> from the end of the address space.  It doesn't usually happen on real
> >> systems, but that's irrelevant.
> >
> > Not a buffer that is 12 bytes long, no. As I said, the critical point
> > is that there is padding after p_end, so p + 6 is still inside the buffer
> > even if it is beyong p_end, thus it is valid C and must work correctly.
> 
> Are you referring to FF_INPUT_BUFFER_PADDING_SIZE?  I hadn't thought of
> that requirement.

Yes, precisely. It's not really a bad idea to not rely on it long as it
doesn't hurt performance, I suspect there are applications out there that
still forget to add it for subtitles (for video most noticed and fixed it
due to the crashes at some point).