On December 2, 2003 09:35 am, Richard Levitte - VMS Whacker wrote: > geoff> afterwards by the memcpy(), and likewise the use of > geoff> sizeof(b->d[0]) seems strange given the logic in the previous > geoff> line uses sizeof(BN_ULONG) (which is better). But "unbelievably > geoff> buggy"? How so? > > A missing 'if (b->d != NULL)' before the memcpy(), corresponding to > the 'if (B != NULL)' higher up.
Oh right, I see. However that too is a bit silly, because a better check would be "if (b->top > 0)". 'dmax' and 'd' could be non-[zero|NULL] even if 'top' isn't. > So OK, it's perhaps not that buggy... :-) Well, it's ugly at the very least. I've been taking the liberty thus far to de-uglify code in some places as I'm going along, but at the same time I'm trying to the scope of my changes under control for the purposes of this "audit". Once I'm done with my stuff (and Nils and Ulf are done with theirs), I think another sweep through the code to sanitise stuff like this will be in order. > geoff> (In fact, the one recurring "bug" to me is the continual use of > geoff> (words+1) when the caller has clearly expressed an interest in > geoff> (words) instead ... > > I think that's plain paranoia, as in "better allocate some space so > off-by-one errors don't bite us in the ass". Well I think "plain paranoia" is better placed in locations like the BN_DEBUG[_RAND] macros followed by the prudent use of self-tests, valgrind, etc. When I do that "second sweep" I spoke of before, I will try pick up on this and things like it. IMHO we're better to convert those (words+1) occurances back to (words) and use debugging to watch for overruns, ie. to fix the cause rather than the symptom. Allocating excess data solves off-by-one only on the assumption that off-by-n for n>1 will never occur, and of course this also introduces bloat whenever "words" is some nicely block-aligned value that we then screw up with an increment. This is analogous to the bn_fix_top() nonsense we had before, where fix-up macros were smeared liberally all over the place so that buggy behaviour was "hidden well enough". Any paranoia that hides bugs is doing us no good. > geoff> another thing to look into at another time - and please don't > geoff> mess with this in the mean time, I've got too many diffs lying > geoff> round already and this could upset my audit apple-cart :-) > > Is this when I should go "Mwahahahahaha", and then not do anything, > leaving you to wonder what you missed? :-) Let's see ... how do I combine "openssl rand -base64", xargs, and a sed for "VMS"? ... [1] :-) Cheers, Geoff [1] PS: I was thinking instead that I should seek retribution using a code obfuscator that I found lying around ... however I then I realised it was actually an SSLeay tree ... -- Geoff Thorpe [EMAIL PROTECTED] http://www.geoffthorpe.net/ ______________________________________________________________________ OpenSSL Project http://www.openssl.org Development Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED]
