On Mon 27 Aug 2012 17:08:48 Niels Thykier escribió:
[snip]
Hi Niels! Sune Vuorela asked with upstream and they both reviewed the code to
find that, while it was not very clear, it's actually safe.
> > ++ // do not overflow the allocated buffer grupx
> > ++ if (offset + cbUPX > grupxLen) {
>
> ^^^^^^^^^^^^^^^^^^^^^^^^^
>
> In my experience it is either
>
> if (offset + i < limit) { /* safe */ }
>
> or
>
> if (offset + i >= limit) { /* abort */ }
>
> Is "offset + cbUPX == grupxLen" really a "safe" index?
OK, let's suppose "offset + cbUPX == grupxLen". Then...
> > ++ wvlog << "====> Error: grupx would overflow!" << endl;
> > ++ return false;
> > ++ }
> > + for ( U16 j = 0; j < cbUPX; ++j ) {
>
> ^^^^^^^^^
>
> This suggests it might not be...
>
> > + grupx[ offset + j ] = stream->readU8(); // read the whole
In the above line, j would reach as maximum (cbUPX - 1), as the above for has
"<" and not "<=".
In the next cupx for() loop, if (offset + cbUPX > grupxLen) will trigger.
So, this seems safe.
Kinds regards, and thanks for pointing this out :-)
Lisandro.
--
porque no respeta el orden natural en el que se leen las cosas
>¿por qué top-posting es tan molesto?
>>top-posting
>>>¿cuál es la peor molestia en los emails de respuesta?
Lisandro Damián Nicanor Pérez Meyer
http://perezmeyer.com.ar/
http://perezmeyer.blogspot.com/
Attachment:
signature.asc
Description: This is a digitally signed message part.