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.