[FFmpeg-devel] [PATCH] Try to fix issue 1973

Reimar Döffinger Reimar.Doeffinger
Fri Feb 18 19:28:41 CET 2011


On Fri, Feb 18, 2011 at 07:02:35PM +0100, Jean-Daniel Dupas wrote:
> Le 18 f?vr. 2011 ? 18:34, Reimar D?ffinger a ?crit :
> > On Fri, Feb 18, 2011 at 10:52:07AM +0100, Jean-Daniel Dupas wrote:
> >> I would like to try to resubmit an old patch that try to fix a buffer overrun in the targa decoder.
> >> It was rejected long ago after endless discussion about  white space and formatting.
> >> 
> >> I tried to fix all formatting issue, but if it remains one, don't bother to tell me, I'm not going to lose more time with this.
> >> If you still prefer to reject a security fix because of a spurious white space, this is your choice.
> >> If there is technical issue though, I'll be glad to fix them.
> > 
> > I am sorry for your bad experience.
> > I am afraid that I think there is a technical issue:
> >> if(buf + needed > buf_end){
> > buf + needed can overflow, thus incorrectly passing this check.
> > The simple rule is to always but the thing to be validated
> > on its own, i.e.
> > if (needed > buf_end - buf)
> > (note I still assume that needed cannot become negative).
> 
> Nice catch. I attach a version with the proposed change.

I think it's good.
It's really unrelated to this (AFAICT you do not and should not change this within
this patch), but in principle it would be more correct to return something like
"avpkt->size - (buf_end - buf)" to reflect the amount of data really used.



More information about the ffmpeg-devel mailing list