[openssl-project] Removal of NULL checks

Andy Polyakov appro at openssl.org
Thu Aug 9 10:29:43 UTC 2018


> Should not be changed period.  Even across major release boundaries.
> This is not an ABI compatibility issue, it is a source compatibility
> issue, and should avoided all the time.  If we want to write a *new*
> function that skips the NULL checks it gets a new name.

While I admittedly crossed a line, I would actually argue against
drawing the line as categorically as above. In sense that NULL checks
*can* be excessive, and thing is that such checks can jeopardize
application security. And in such case it wouldn't be totally
inappropriate to remove them, excessive checks, *without* renaming
function as suggested above. It would be kind of equivalent of changing
one undefined behaviour pattern for another undefined one. Or rather for
more "honest" undefined behaviour pattern:-)

As for jeopardizing application security, taking this case as an
example. What worked so far? Create stack without paying attention to
result[!], dump things into stack even if it's NULL, pull nothing if
it's NULL. Now imagine that this stack was some kind of deny list. If
entry producer didn't pay attention to error when creating stack and
putting things into it, consumer would be led to belief that there is
nothing to deny and let through the requests that are supposed to be
denied. This is kind of not-quite-right example in the context, because
it implies that *all* NULL checks should have been removed, thus making
*every* caller to pay attention, including consumer. While I left checks
in some of the consume operations reckoning that at least those who will
be putting things into NULL stack will have to pay attention and will
cancel whole operation hopefully without getting to pull-nothing step.
So that those entry producers who are not *currently* paying attention
would actually be caught. Which actually would be a positive thing!

Oh! Triggering factor was quite a number of unchecked pushes in apps.
Yes, you can sense contradiction in sense that one can't "fix" those
unchecked pushes with removal of NULL checks in questions. But they were
just something that made me look at stack.c and wonder above questions.


More information about the openssl-project mailing list