From 815822d81cd69814927013d24133aa2a0baa6179 Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Sun, 20 Nov 2016 11:47:09 +0100 Subject: [PATCH 1/9] don't arbitrarily limit UIDs to a billion the number was chosen to make queries more comprehensible when the server sends no UIDNEXT, but it appears that such insanely large UIDs actually show up in the wild. so send 32-bit INT_MAX instead. note that this is again making an assumption: that no server uses unsigned ints for UIDs. but we can't sent UINT_MAX, as that would break with servers which use signed ints. also, *we* use signed ints (which is actually a clear violation of the spec). it would be possible to special-case the range [1,inf] to 1:*, thus entirely removing arbitrary limits. however, when the range doesn't start at 1, we may actually get a single message instead of none due to the imap uid range limits being unordered. this gets really nasty when we need to issue multiple queries, as we may list the same message twice. a reliable way around this would be issuing a separate query to find the actual value of UID '*', to make up for the server not sending UIDNEXT in the first place. this would obviously imply an additional round-trip per mailbox ... --- src/drv_imap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/drv_imap.c b/src/drv_imap.c index 608a2d3..4051ec6 100644 --- a/src/drv_imap.c +++ b/src/drv_imap.c @@ -2300,7 +2300,7 @@ imap_load_box( store_t *gctx, int minuid, int maxuid, int newuid, int *excs, int imap_submit_load( ctx, buf, 0, sts ); } if (maxuid == INT_MAX) - maxuid = ctx->gen.uidnext ? ctx->gen.uidnext - 1 : 1000000000; + maxuid = ctx->gen.uidnext ? ctx->gen.uidnext - 1 : 0x7fffffff; if (maxuid >= minuid) { if ((ctx->gen.opts & OPEN_FIND) && minuid < newuid) { sprintf( buf, "%d:%d", minuid, newuid - 1 ); From bc51d0206aa72c97f79b21a00173edb059c610dd Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Sat, 3 Dec 2016 14:32:51 +0100 Subject: [PATCH 2/9] fix LOGIN in non-SASL builds specifically, if AuthMechs included more than just LOGIN (which would be the case for '*') and the server announced any AUTH= mechanism, we'd immediately error out upon seeing it, thus failing to actually try LOGIN. --- src/drv_imap.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/drv_imap.c b/src/drv_imap.c index 4051ec6..4bc2fcc 100644 --- a/src/drv_imap.c +++ b/src/drv_imap.c @@ -1957,17 +1957,14 @@ imap_open_store_authenticate2( imap_store_t *ctx ) if (ctx->conn.ssl || !any) #endif auth_login = 1; - } else { #ifdef HAVE_LIBSASL + } else { int len = strlen( cmech->string ); if (saslend + len + 2 > saslmechs + sizeof(saslmechs)) oob(); *saslend++ = ' '; memcpy( saslend, cmech->string, len + 1 ); saslend += len; -#else - error( "IMAP error: authentication mechanism %s is not supported\n", cmech->string ); - goto bail; #endif } } From 2f91e22371fb2b7054e1576f6f85bde429cefec5 Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Sat, 3 Dec 2016 20:58:16 +0100 Subject: [PATCH 3/9] fix LOGIN in SASL builds if AuthMechs includes more than just LOGIN and the server announces any AUTH= mechanism, we try SASL. but that can still fail to find any suitable authentication mechanism, and we must not error out in that case if we are supposed to fall back to LOGIN. --- src/drv_imap.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/drv_imap.c b/src/drv_imap.c index 4bc2fcc..5d77f08 100644 --- a/src/drv_imap.c +++ b/src/drv_imap.c @@ -1992,6 +1992,8 @@ imap_open_store_authenticate2( imap_store_t *ctx ) rc = sasl_client_new( "imap", srvc->sconf.host, NULL, NULL, NULL, 0, &ctx->sasl ); if (rc != SASL_OK) { + if (rc == SASL_NOMECH) + goto notsasl; if (!ctx->sasl) goto saslbail; error( "Error: %s\n", sasl_errdetail( ctx->sasl ) ); @@ -1999,6 +2001,8 @@ imap_open_store_authenticate2( imap_store_t *ctx ) } rc = sasl_client_start( ctx->sasl, saslmechs + 1, &interact, CAP(SASLIR) ? &out : NULL, &out_len, &gotmech ); + if (rc == SASL_NOMECH) + goto notsasl; if (gotmech) info( "Authenticating with SASL mechanism %s...\n", gotmech ); /* Technically, we are supposed to loop over sasl_client_start(), @@ -2017,6 +2021,8 @@ imap_open_store_authenticate2( imap_store_t *ctx ) imap_exec( ctx, cmd, done_sasl_auth, enc ? "AUTHENTICATE %s %s" : "AUTHENTICATE %s", gotmech, enc ); free( enc ); return; + notsasl: + sasl_dispose( &ctx->sasl ); } #endif if (auth_login) { From fdb03b91f2471ebc1ee715c78911c8a1085791d1 Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Sat, 3 Dec 2016 20:58:23 +0100 Subject: [PATCH 4/9] be more helpful when no SASL mechanisms are available --- src/drv_imap.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/drv_imap.c b/src/drv_imap.c index 5d77f08..9d7c824 100644 --- a/src/drv_imap.c +++ b/src/drv_imap.c @@ -1944,6 +1944,7 @@ imap_open_store_authenticate2( imap_store_t *ctx ) string_list_t *mech, *cmech; int auth_login = 0; #ifdef HAVE_LIBSASL + const char *saslavail; char saslmechs[1024], *saslend = saslmechs; #endif @@ -2022,6 +2023,14 @@ imap_open_store_authenticate2( imap_store_t *ctx ) free( enc ); return; notsasl: + if (!ctx->sasl || sasl_listmech( ctx->sasl, NULL, "", "", "", &saslavail, NULL, NULL ) != SASL_OK) + saslavail = "(none)"; /* EXTERNAL is always there anyway. */ + if (!auth_login) { + error( "IMAP error: selected SASL mechanism(s) not available;\n" + " selected:%s\n available: %s\n", saslmechs, saslavail ); + goto bail; + } + info( "NOT using available SASL mechanism(s): %s\n", saslavail ); sasl_dispose( &ctx->sasl ); } #endif From 1b235d3d466afea7d41d60d12fb96c4a83b2671e Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Sat, 3 Dec 2016 20:00:38 +0100 Subject: [PATCH 5/9] make * not match LOGIN even in non-SSL builds this is consistent with the plain text transmission warning below. --- src/drv_imap.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/drv_imap.c b/src/drv_imap.c index 9d7c824..e91ca36 100644 --- a/src/drv_imap.c +++ b/src/drv_imap.c @@ -1956,6 +1956,8 @@ imap_open_store_authenticate2( imap_store_t *ctx ) if (!strcasecmp( cmech->string, "LOGIN" )) { #ifdef HAVE_LIBSSL if (ctx->conn.ssl || !any) +#else + if (!any) #endif auth_login = 1; #ifdef HAVE_LIBSASL From 1a707ab1563c8651e5ced0babf344baab6dad039 Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Sat, 3 Dec 2016 19:18:12 +0100 Subject: [PATCH 6/9] inform user if LOGIN was skipped because of missing SSL 'AuthMechs *' technically includes LOGIN, so it is a bit unintuitive when it's still not used. --- src/drv_imap.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/drv_imap.c b/src/drv_imap.c index e91ca36..686faef 100644 --- a/src/drv_imap.c +++ b/src/drv_imap.c @@ -1943,6 +1943,7 @@ imap_open_store_authenticate2( imap_store_t *ctx ) imap_server_conf_t *srvc = cfg->server; string_list_t *mech, *cmech; int auth_login = 0; + int skipped_login = 0; #ifdef HAVE_LIBSASL const char *saslavail; char saslmechs[1024], *saslend = saslmechs; @@ -1960,6 +1961,8 @@ imap_open_store_authenticate2( imap_store_t *ctx ) if (!any) #endif auth_login = 1; + else + skipped_login = 1; #ifdef HAVE_LIBSASL } else { int len = strlen( cmech->string ); @@ -2030,7 +2033,7 @@ imap_open_store_authenticate2( imap_store_t *ctx ) if (!auth_login) { error( "IMAP error: selected SASL mechanism(s) not available;\n" " selected:%s\n available: %s\n", saslmechs, saslavail ); - goto bail; + goto skipnote; } info( "NOT using available SASL mechanism(s): %s\n", saslavail ); sasl_dispose( &ctx->sasl ); @@ -2048,6 +2051,12 @@ imap_open_store_authenticate2( imap_store_t *ctx ) return; } error( "IMAP error: server supports no acceptable authentication mechanism\n" ); +#ifdef HAVE_LIBSASL + skipnote: +#endif + if (skipped_login) + error( "Note: not using LOGIN because connection is not encrypted;\n" + " use 'AuthMechs LOGIN' explicitly to force it.\n" ); bail: imap_open_store_bail( ctx, FAIL_FINAL ); From 03e25db3b8e5e938f1bb1009d747514b1315cae7 Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Sun, 4 Dec 2016 11:14:34 +0100 Subject: [PATCH 7/9] validate NAMESPACE response earlier ... and don't silently fail later on. --- src/drv_imap.c | 37 ++++++++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/src/drv_imap.c b/src/drv_imap.c index 686faef..f15bc5b 100644 --- a/src/drv_imap.c +++ b/src/drv_imap.c @@ -837,33 +837,50 @@ static int parse_namespace_rsp_p2( imap_store_t *, list_t *, char * ); static int parse_namespace_rsp_p3( imap_store_t *, list_t *, char * ); static int -parse_namespace_rsp_fail( void ) +parse_namespace_check( list_t *list ) { + if (!list) + goto bad; + if (list->val == NIL) + return 0; + if (list->val != LIST) + goto bad; + for (list = list->child; list; list = list->next) { + if (list->val != LIST) + goto bad; + if (!is_atom( list->child )) + goto bad; + if (!is_atom( list->child->next )) + goto bad; + /* Namespace response extensions may follow here; we don't care. */ + } + return 0; + bad: error( "IMAP error: malformed NAMESPACE response\n" ); - return LIST_BAD; + return -1; } static int parse_namespace_rsp( imap_store_t *ctx, list_t *list, char *s ) { - if (!(ctx->ns_personal = list)) - return parse_namespace_rsp_fail(); + if (parse_namespace_check( (ctx->ns_personal = list) )) + return LIST_BAD; return parse_list( ctx, s, parse_namespace_rsp_p2 ); } static int parse_namespace_rsp_p2( imap_store_t *ctx, list_t *list, char *s ) { - if (!(ctx->ns_other = list)) - return parse_namespace_rsp_fail(); + if (parse_namespace_check( (ctx->ns_other = list) )) + return LIST_BAD; return parse_list( ctx, s, parse_namespace_rsp_p3 ); } static int parse_namespace_rsp_p3( imap_store_t *ctx, list_t *list, char *s ATTR_UNUSED ) { - if (!(ctx->ns_shared = list)) - return parse_namespace_rsp_fail(); + if (parse_namespace_check( (ctx->ns_shared = list) )) + return LIST_BAD; return LIST_OK; } @@ -2142,10 +2159,8 @@ imap_open_store_namespace2( imap_store_t *ctx ) ctx->prefix = nsp_1st_ns->val; if (!ctx->delimiter) ctx->delimiter = nfstrdup( nsp_1st_dl->val ); - imap_open_store_finalize( ctx ); - } else { - imap_open_store_bail( ctx, FAIL_FINAL ); } + imap_open_store_finalize( ctx ); } static void From ef0e7fdd3e921d612220a830436bb9862bca61ac Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Sun, 4 Dec 2016 11:23:47 +0100 Subject: [PATCH 8/9] accept NAMESPACE responses without hierarchy delimiter RFC2342 states that the delimiter may be NIL, which some servers apparently actually make use of. REFMAIL: CAM0xXk_FQ83CPrd37iQCMKtc1B2P8=u-r5jX0n2WE5Y+3483nQ@mail.gmail.com --- src/drv_imap.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/drv_imap.c b/src/drv_imap.c index f15bc5b..83b0358 100644 --- a/src/drv_imap.c +++ b/src/drv_imap.c @@ -637,6 +637,12 @@ next_arg( char **ps ) return ret; } +static int +is_opt_atom( list_t *list ) +{ + return list && list->val && list->val != LIST; +} + static int is_atom( list_t *list ) { @@ -850,7 +856,7 @@ parse_namespace_check( list_t *list ) goto bad; if (!is_atom( list->child )) goto bad; - if (!is_atom( list->child->next )) + if (!is_opt_atom( list->child->next )) goto bad; /* Namespace response extensions may follow here; we don't care. */ } @@ -2147,17 +2153,17 @@ static void imap_open_store_namespace2( imap_store_t *ctx ) { imap_store_conf_t *cfg = (imap_store_conf_t *)ctx->gen.conf; - list_t *nsp, *nsp_1st, *nsp_1st_ns, *nsp_1st_dl; + list_t *nsp, *nsp_1st; /* XXX for now assume 1st personal namespace */ if (is_list( (nsp = ctx->ns_personal) ) && - is_list( (nsp_1st = nsp->child) ) && - is_atom( (nsp_1st_ns = nsp_1st->child) ) && - is_atom( (nsp_1st_dl = nsp_1st_ns->next) )) + is_list( (nsp_1st = nsp->child) )) { + list_t *nsp_1st_ns = nsp_1st->child; + list_t *nsp_1st_dl = nsp_1st_ns->next; if (!ctx->prefix && cfg->use_namespace) ctx->prefix = nsp_1st_ns->val; - if (!ctx->delimiter) + if (!ctx->delimiter && is_atom( nsp_1st_dl )) ctx->delimiter = nfstrdup( nsp_1st_dl->val ); } imap_open_store_finalize( ctx ); From 743968737c5e296fb5c7d0d2dc1de091cc84c0d7 Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Sun, 11 Dec 2016 12:09:36 +0100 Subject: [PATCH 9/9] silence bogus [-Wmaybe-uninitialized] with -O0/-O1/-Os --- src/util.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/util.c b/src/util.c index f05eec6..3d2d5fc 100644 --- a/src/util.c +++ b/src/util.c @@ -522,7 +522,15 @@ map_name( const char *arg, char **result, int reserve, const char *in, const cha for (ll = 0; ll < inl; ll++) if (arg[i + ll] != in[ll]) goto rnexti; +#ifdef __GNUC__ +# pragma GCC diagnostic push +/* https://gcc.gnu.org/bugzilla/show_bug.cgi?id=42145 */ +# pragma GCC diagnostic ignored "-Wmaybe-uninitialized" +#endif memcpy( p, out, outl ); +#ifdef __GNUC__ +# pragma GCC diagnostic pop +#endif p += outl; i += inl; continue;