On Wed, Jul 13, 2016 at 08:05:03PM -0500, Will Fiveash wrote: > 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"?
Perhaps Mutt only works with the Cyrus implementation of SASL, but I was worried there might be another one with different behavior. On the other hand I do see the documentation in Cyrus SASL say the rule is "if you allocate it, you free it", so maybe I'm worrying about nothing. > + static unsigned char passwd_buf[520]; I'd really rather we not cap the size like this (especially after I had to do https://dev.mutt.org/hg/mutt/rev/b315c4d4ede7). If we're going to fix this, I'd rather safe_realloc() it each time. > /* *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); There is a len field, but I'd also still like to see the account->pass nul-terminated. I see plugins/plugin_common.c doing that in the sasl source (when they prompt, not when they call the callback), so it's probably a good idea. -- Kevin J. McCarthy GPG Fingerprint: 8975 A9B3 3AA3 7910 385C 5308 ADEF 7684 8031 6BDA
signature.asc
Description: PGP signature
