Subject: DIAGASSERT in cryptographic functions.
To: None <lukem@netbsd.org>
From: Bill Sommerfeld <sommerfeld@orchard.arlington.ma.us>
List: tech-userlevel
Date: 09/16/1999 12:43:52
I noticed that your DIAGASSERT changes include code like:

+#ifdef _DIAGNOSTIC
+       if (context == 0 || input == 0)
+               return;
+#endif

in md5c.c::MD5Final(), which may cause the function to silently return a
potentially erroneous value to its caller.

This is especially troublesome in the case of MD5Final(), since it
means that a bug which would previously have been fatal will now no
longer be (in the presence of a DIAGASSERT runtime), and the program
will continue running and generate possibly erroneous results (for
instance, if context were NULL, the program might make a
security-critical decision based on the (uninitialized) contents of
the digest output).

it's clear that there's a difference in philosophy here..

My experience has taught me that the best way to keep defects from
piling up in the development cycle (which is where tools like
DIAGASSERT are most useful) is to make detected bugs *hurt* (and cause
the program to abort and dump core); the alternative is that buggy
programs keep running and often destroy the evidence of where the
failure came from.

I think the whole "try to keep running in the face of detected errors"
aspect of the _DIAGASSERT/_DIAGNOSTIC changes is ill-advised; the
#ifdef _DIAGNOSTIC/if (...)  return;/#endif blocks should be deleted,
and _DIAGASSERT should be changed to cause a fatal exception rather
than merely a warning.


					- Bill