On Tue, Jul 12, 2016 at 07:02:01PM -0700, Kevin J. McCarthy wrote:
> It looks like the SASL password is leaked, but it's not clear to me how
> we're supposed to free that.  I saw one of the example clients used a
> static variable and reused it for each authentication.  I'm reluctant to
> make that change though: I'd rather leak the memory than have an
> implementation try to free it and then we realloc the freed memory.

What do you mean by "an implementation"?  Note that on Solaris using
Cyrus libsasl I made the following changes to mutt_sasl.c:

+ static unsigned char passwd_buf[520];

and in mutt_sasl_cb_pass():

  len = strlen (account->pass);
  if (len > 512) {
    return SASL_FAIL;
  }

  /* *psecret = (sasl_secret_t*) safe_malloc (sizeof (sasl_secret_t) + len); */
  /* (*psecret)->len = len; */
  /* strcpy ((char*)(*psecret)->data, account->pass); *//* __STRCPY_CHECKED__ */
  memcpy(((sasl_secret_t *) passwd_buf)->data, account->pass, (size_t) len);
  ((sasl_secret_t *) passwd_buf)->len = len;
  *psecret = (sasl_secret_t *) passwd_buf;

  return SASL_OK;
}

I tested sending via libsasl->SSL with the Solaris libumem debugging
enabled which will catch invalid free()'s, double free()'s, memory
leaks, etc... and the change above appears to not cause any memory
corruption and the leak is gone.

-- 
Will Fiveash

Reply via email to