Hi,
Rainer Zocholl wrote:
[snip]
>>As a result, the expected length evaluates to 6 bytes, but "length"
>>containes already more than that (in the above case it was 79). This
>>finally leads to "Rest" beeing -73 which then causes a crash in
>>memcpy.
>
> Nice bug, nice analysis.
>
> but that leds to a general question:
> Why are "foreign"-based parameters to memcpy not range checked?
> Nobody should trust any parameters he got from "outside",
> we learned in grammar school.
>
> Such "out of range" writings might cause very funny,
> hard to debug errors. Mostly not so "lulely" crashes the
> box so reproducible.
>
> Are there maybe more place where "memcpy" and similar critical
> ("buffer overflow sensitive") function are used with maybe "tainted"
> unchecked values, which assumes that all previous calculations are
> done right and all parameters "mets" the specs?
Well, as I wrote, it took the whole afternoon and evening to find the
bug. I've read the code over and over and didn't find any case, where it
would result in a negative length for memcpy(), even if the processed
data would not match the specs. It was simply a programming error that
dropped part of the data and resulted in a crash later.
So I don't think that each and every memcpy() should be protected in
general. In this case, the code properly takes care of finding a valid
length for memcpy(), so it should be sufficient to verify that the code
operates properly, maybe by adding an assert() statement.
Anything else wouldn't make sense in that case (just my opinion).
Bye.
--
Dipl.-Inform. (FH) Reinhard Nissl
mailto:rnissl@xxxxxx
