[FFmpeg-devel] ADPCM task (was Re: files in incoming)

[Måns Rullgård]

Stefan Gehrer <stefan.gehrer at gmx.de> writes:

> Reimar D?ffinger wrote:
>> On Fri, Jan 30, 2009 at 06:55:04PM +0100, Stefan Gehrer wrote:
>>> Reimar D?ffinger wrote:
>>>> On Fri, Jan 30, 2009 at 08:06:04AM +0100, Stefan Gehrer wrote:
>>>>> @@ -1303,6 +1304,7 @@
>>>>>              srcC  = src + (avctx->channels-channel) * 4;
>>>>>              srcC += (big_endian ? bytestream_get_be32(&src)
>>>>>                                  : bytestream_get_le32(&src));
>>>>> +            if ((srcC > src_end - 4) || (srcC < src)) break;
>>>> Unfortunately no, a C compiler is allowed to assume that pointer
>>>> operations will never overflow, thus removing the (srcC < src) check.
>>> Interesting. Do you have a source where I can read that up?
>>> And if the answer is ANSI C / ISO 9899, maybe a more specific hint?
>> 
>> Well, the way I put it is the reality as gcc handles it, the standard is
>> far more restrictive, quoting the C99 standardi, section 6.5.6.8:
>>> If both the pointer
>>> operand and the result point to elements of the same array object, or
>>> one past the last
>>> element of the array object, the evaluation shall not produce an
>>> overflow; otherwise, the
>>> behavior is undefined.
>> 
>> Emphasis on "undefined", which is about the worst case you can
>> encounter.
>> So even as soon as you just add a value that is larger than
>> the array size to a pointer anything may happen just going by this...
>> In case of gcc, "only" an important half of your security check goes
>> missing.
>
> Hmm, I am not entirely convinced. For me that rather reads
> "if you do your pointer arithmetics nicely within an array, their
> will be no overflow. If you do it beyond an array, there may
> or may not be an overflow."
> That means after our calculation of srcC there might have
> been an overflow, as it is not pointing to an element of an array
> object, therefore a check on srcC being smaller than src is a valid
> check that can not be omitted.
> But of course I am happy to be corrected ...

Pointer arithmetic resulting in a pointer not within or immediately
beyond the array has undefined behaviour.  This includes doing nothing
at all.  For srcC < src to be true, an overflow must have occurred.
The standard states that arithmetic within the defined bounds does not
overflow, hence an operation with undefined behaviour must have been
performed.  Whatever that operation was, it isn't required to overflow
(it's undefined), and the compiler is free to do anything, including
dropping the check.

-- 
M?ns Rullg?rd
mans at mansr.com