[Ffmpeg-devel] [PATCH] THP PCM decoder (GSoC Qualification)

[Michael Niedermayer]

Hi

On Fri, Apr 06, 2007 at 02:00:30PM +0200, Marco Gerards wrote:
> Michael Niedermayer <michaelni at gmx.at> writes:
> 
> > Hi
> >
> > On Fri, Apr 06, 2007 at 11:30:07AM +0200, Marco Gerards wrote:
> >> Michael Niedermayer <michaelni at gmx.at> writes:
> >> 
> >> Hi,
> >> 
> >> >> Or what are the bugs you mean?
> >> >
> >> > buffer overflow / segfault / exploit / ...
> >> 
> >> Oh, I multiplied with st, but I should have multiplied with (st + 1).
> >
> > yes
> >
> >
> >> I have included a new patch.  If there are still bugs on this single
> >> line, I either don't understand what you mean or I just don't see it
> >> because I am misunderstanding something.
> >> 
> >> What I currently have is:
> >> +        if (samples + samplecnt * (st + 1) >= samples_end) {
> >
> > that contains one bug and a fairly serious one, it still doenst
> > catch all buffer overflow cases
> >
> >
> >> 
> >> I read this as: if (address_of_last_sample >= last_address_of_buffer_plus_one) {
> >
> > this line is equivalent to the one above so it also contains the bug
> 
> In that case I simply don't see it.  If this check evaluates to false,
> the last sample is stored before the end of the output buffer.
> Perhaps even in the last 16 bits.  But in that case there is no buffer
> overflow.
> 
> Buffer address: addr
> Buffer size: 0x0800
> 
> For example: address_of_last_sample = addr+0x7fe
>     last_address_of_buffer_plus_one = addr+0x800
> 
> In this case, nothing wrong happened, addr+0x7fe and addr+0x7ff are the last
> bytes in this buffer and are perfectly valid.
> 
> Just when address_of_last_sample >= addr+0x800, there is a buffer
> overflow.
> 
> But obviously this is wrong, otherwise you wouldn't tell me there is a
> bug in this line.  Could you please tell me what the problem is, so I
> can properly fix this and avoid mistakes like this in future patches?

heres a tip, i claim the line is buggy that is it will segfault when 
the samples are written you dont see how that can be, so why dont
you just test the code in a loop with:

samplecnt= random() betweeen 0..0xFFFFFFFF

if it doesnt segfault iam wrong, if it does you should be able to figure
out why 

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I do not agree with what you have to say, but I'll defend to the death your
right to say it. -- Voltaire
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070406/71eed9a0/attachment.pgp>