[openssl-project] Removal of NULL checks

[Viktor Dukhovni]

On Thu, Aug 09, 2018 at 06:40:14PM +0200, Richard Levitte wrote:
> In message <20180809162245.GD14409 at straasha.imrryr.org> on Thu, 9 Aug 2018 12:22:45 -0400, Viktor Dukhovni <openssl-users at dukhovni.org> said:
> 
> openssl-users> It needs to be possible to recompile and run without auditing code.
> openssl-users> The worst kind of incompatibilities are those that are not reported
> openssl-users> by the compiler, and are only found at runtime, possibly under unusual
> openssl-users> conditions.
> 
> So in this particular case, such as unchecked calls of sk_ functions,
> including sk_TYPE_new(), just to discover later that "oops, the
> elements we thought we inserted aren't there"?  ;-)

The NULL checks were returning an error, not silently failing to
add the element.

> Either way, sk == NULL will not be reported by the compiler, will only
> be found at runtime, possibly under unusual conditions.

Code that tested for the error and avoided a crash would absent
NULL checks crash (in unexpected cases).  The existing code should
be assumed to be correctly written for the current library behaviour.
What happens to already broken code is of little interest.

> The only
> difference is exactly how the user gets to find out in runtime; 1)
> mysterious failures because the stack that should contain n elements
> is really empty and unfillable, or 2) an immediate crash.

This is simply wrong I'm afraid.  The NULL checks in question
returned an *error* (rather than crash) that the application should
check.  Applications that are not doing their own NULL check and
expect us to do it for them, can check for return value.  This
makes possible more concise code:

	X509 *x;
	STACK_OF(X509) *s;

	...
	/* Allocate 's' and initialize with x as first element */
	if (sk_X509_push(s = sk_X509_new(NULL), x) < 0) {
		/* error */
	}
	/* s is stack holding x */
	...

-- 
	Viktor.