On September 25, 2003 03:33 am, Nils Larsch wrote: > Otto Moerbeek wrote: > .... > > > OK, that would amount to the fixes below: > > > > - in BN_cmp, call bn_fix_top just before comparing the two tops. > > Not really necessary as the normal BN_* functions which change the > value of the bignum should always ensure that the top value is > correct (i.e. as small as possible), but of course adding bn_fix_top > does not really harm (besides wasting some cpu time :-) .
I would go one step further and suggest that BN_cmp() and operations like it that should treat their inputs as "const" should not modify the BIGNUMs at all. Fixing BIGNUMs wherever we spot problems helps propogate the current mess, the correct fix (as Nils implied) is to trace the problem back to where it is caused and fix it there. No BIGNUM structure should ever be in an invalid state outside the scope of a BN_*** API function (or macro). So each function should, on exit, have correctly adjusted any data as necessary. Perhaps (any volunteers?) the best way to do this would be to define a validation function/macro and call it from every exit path for every BIGNUM API function and macro. This could sanity-check the in/out variables and abort() if there's something wrong so that the debugger can catch these problems where they occur rather than where we first notice them (later on in things like BN_cmp). IMHO this would be infinitely more useful than continually adding calls to bn_fix_top() everywhere it seems it could seal off another "case". > Perhaps you should send a bug report to [EMAIL PROTECTED] containing the > patch and a link to this thread. Yup, this should certainly go into RT - but I think this would be the ideal opportunity to audit the BIGNUM code properly for this sort of thing. In particular, it would be nice if the result of this work would also allow us to better constify the BIGNUM API (in CVS HEAD of course, the stable branches would merely benefit from cleaner implementations;-). Cheers, Geoff -- Geoff Thorpe [EMAIL PROTECTED] http://www.geoffthorpe.net/ ______________________________________________________________________ OpenSSL Project http://www.openssl.org Development Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED]
