Fix a series of security issues

[alex]

See individual commits for details

[shinmao]

Hi, if we want to avoid null pointer and initialize the ctx, why not using MaybeUninit instead of mem::zeroed based on the description in documentation:

This has the same effect as MaybeUninit::zeroed().assume_init(). It is useful for FFI sometimes, but should generally be avoided.

Even though ctx is not a reference here, it is still better to use MaybeUninit, isn't it?

[alex]

Probably using MaybeUnit consistently would be better, yeah. Would you like to submit a PR to do so?

[shinmao]

I would love to do it, but I am still a bit confused about the root cause.

If I understand correctly, null ptr dereference happens because context was sent to C function ffi::X509V3_EXT_nconf as a null mutable pointer. However, based on the codebase of openssl crate, it is a common behavior to pass null_ptr directly to the C function.

I believe there could be another null pointer deref issue hidden in the crate. Is there any suggestion to avoid such issue?

[alex]

The null-pointer derefs are inside of OpenSSL, it wasn't a crash in Rust itself. The problem is that some extensions require a context value, but others allow null just fine.

There's test cases for it.

[shinmao]

Yes, I think my description was not precise enough. "dereference" part was in OpenSSL(C), and "null-pointer" part was led by None => null_mut in Rust itself.

However, what I am confused is that: Why is it Rust's responsibility to fix the bug? null-pointer derefs happened in C code, then X509V3_EXT_nconf should check whether context value is null for some extensions, isn't it?

[sfackler]

This is a safe Rust function. It should not be possible to cause a null pointer deref from calling it.