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"? 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():
I refactored my changes and attached the diff.
--
Will Fiveash
diff --git a/mutt_sasl.c b/mutt_sasl.c
--- a/mutt_sasl.c
+++ b/mutt_sasl.c
@@ -86,6 +86,13 @@
static int mutt_sasl_start (void);
+static struct passwd_buf_struct {
+ unsigned long len;
+ unsigned char data[512];
+} passwd_buf;
+
+static sasl_secret_t *pass_ptr = (sasl_secret_t *) &passwd_buf;
+
/* callbacks */
static int mutt_sasl_cb_log (void* context, int priority, const char* message);
static int mutt_sasl_cb_authname (void* context, int id, const char** result,
@@ -444,10 +451,13 @@
return SASL_FAIL;
len = strlen (account->pass);
+ if (len > sizeof (passwd_buf.data)) {
+ 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(pass_ptr->data, account->pass, (size_t) len);
+ pass_ptr->len = len;
+ *psecret = pass_ptr;
return SASL_OK;
}
