Re: [RFC] imap-send: escape backslash in password

On Fri, Aug 04, 2017 at 08:06:43PM +0000, brian m. carlson wrote:

On Fri, Aug 04, 2017 at 06:16:53PM +0200, Nicolas Morey-Chaisemartin wrote:

quoted

 static struct imap_store *imap_open_store(struct imap_server_conf *srvc, char *folder)
 {
 	struct credential cred = CREDENTIAL_INIT;

@@ -1090,7 +1116,7 @@ static struct imap_store *imap_open_store(struct imap_server_conf *srvc, char *f
 			if (!srvc->user)
 				srvc->user = xstrdup(cred.username);
 			if (!srvc->pass)
-				srvc->pass = xstrdup(cred.password);
+				srvc->pass = imap_escape_password(cred.password);
 		}
 
 		if (srvc->auth_method) {

I'm not sure if this is correct.  It looks like this username and
password are used by whatever authentication method we use, whether
that's LOGIN or CRAM-MD5.  I don't think we'd want to encode the
password here before sending it through the CRAM-MD5 authenticator.

Yeah. This is an on-the-wire encoding issue, and should happen as part
of forming the protocol string to send. So:

  imap_exec(ctx, NULL, "LOGIN \"%s\" \"%s\"", srvc->user, srvc->pass)

is probably where it needs to happen.

It looks like this issue is present in a lot of other places, too. Just
a few lines below I see:

  imap_exec(ctx, NULL, "CREATE \"%s\"", ctx->name)

As an aside, these are all potential injection vulnerabilities, too.
E.g., if I specify my folder as

  foo"\n. DELETE "bar

then we'd issue an accidental deletion. I doubt it's a big deal in
practice, as it's not common to feed attacker-controlled strings to
imap-send. But we should probably fix it anyway.

The right interface is probably to teach imap_exec() to take a
NULL-terminated list of items (rather than a format string) and then
quote each one appropriately.

-Peff