[openssl-dev] [openssl.org #4094] Nonsensical pointer comparison in PACKET_buf_init

[Matt Caswell via RT]

On 15/10/15 14:35, Salz, Rich via RT wrote:
> 
>> PACKET_buf_init. This code can assume that |len| is from a trusted source.
>>
>> The purpose of the sanity check is not then for security, but to guard against
>> programmer error. For a correctly functioning program this test should never
>> fail.
> 
> I would say that the combination of these two things means that it should be an assert.

assert has its uses, but it depends what you are trying to achieve. If
you are developing new code and trying to track down a difficult to find
problem - very useful. Even potentially if you are trying to track down
a difficult issue in a production scenario, you could create a special
debug build to help you pinpoint what is going wrong. I'm sure there are
other scenarios which would be excellent uses of assert.

IMO this test is about protecting ourselves in the unlikely event that a
programmer error does not present itself until production. To protect
production code assert is useless because it gets compiled away.

If we are concerned about potential programmer errors not appearing
until we get into production then assert is not the right tool. You
could argue that you should use OPENSSL_assert(), but my views on that
are well known. OPENSSL_assert calls abort() even in production which is
wrong IMO and potentially dangerous. I know opinions differ on that
point within the dev team and this ticket is probably not the place to
reopen that particular debate. Suffice it to say I would not support the
use of OPENSSL_assert here.

Matt