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

[Marco Gerards]

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?

--
Marco