• Registered: 2020-06-15
  • Posts: 21

Topic: Cleanup bugs in wolfSSL_Init()

In wolfSSL_init(), if initRefCount == 0 when called and when wolfCrypt_Init() returns 0 (succeeds), then after that wolfCrypt_Cleanup() is not called if subsequently any additional error occurs.

Likewise, if the call to wolfSSL_RAND_seed succeeds, but an error happens after that then also wolfSSL_RAND_Cleanup() is not called to clean up.

Finally, if `wc_InitMutex(&session_mutex)` succeeds, but an error happens after that then `wc_FreeMutex` is not called and
if `wc_InitMutex(&count_mutex)` succeeds but `wc_LockMutex(&count_mutex)` fails, then again `wc_FreeMutex` is not called.

Share

  • From: Bozeman
  • Registered: 2014-05-09
  • Posts: 777

Re: Cleanup bugs in wolfSSL_Init()

We will look into it thank you so much for the report!

- KH

Kaleb J. Himes's Website

Share

  • Registered: 2020-06-15
  • Posts: 21

Re: Cleanup bugs in wolfSSL_Init()

Thanks,

I suppose it is debatable how serious failure to cleanup is when there is a fatal error to initialize wolfssl: it that case it is likely that the program will be terminated anyway.

The reason I fell over this however is because I'm integrating wolfssl with my C++ library and found myself calling wolfSSL_init() from the constructor of a class:

TLS::TLS() : m_session(nullptr)
{
  DoutEntering(dc::tls, "TLS::TLS() [" << this << "]");
  std::call_once(s_flag, global_tls_initialization);
}

where

//static
void TLS::global_tls_initialization()
{
  DoutEntering(dc::tls|dc::notice, "evio::protocol::TLS::global_tls_initialization()");

  // Call this to have wolfssl print debug output (wolfssl must be configured with --enable-debug).
  //wolfSSL_Debugging_ON();
  
  Dout(dc::tls|continued_cf, "wolfSSL_Init() = ");
  wolfssl_error_code ret = wolfSSL_Init();
  Dout(dc::finish, ret);
  if (ret != WOLFSSL_SUCCESS)
    THROW_FALERTC(ret, "wolfSSL_Init");

...

At that point, where I possibly throw an exception from a constructor, I feel strongly obliged to make SURE that there are no side effects from the constructor. A constructor that throws should have the effect like it wasn't called at all. This is just a holy rule that is etched into my coding habbits;

so, I naturally looked inside `wolfSSL_Init()` to make sure it had no side effects when it returned something other than WOLFSSL_SUCCESS.

In theory, it would be OK when wolfSSL_Init() had side effect (from the C coding point of view), but in that case it would be nice if it returned an error that allowed me to unwind all the damage. But that is certainly not possible, if only because it returns the same error codes from different stages of initialization.

Share