René Scharfe <l.s.r@xxxxxx> writes:
> @@ -807,6 +816,8 @@ static int get_cmd_result(struct imap_store *ctx, struct imap_cmd *tcmd)
> if (cmdp->cb.cont || cmdp->cb.data)
> imap->literal_pending = 0;
> arg = next_arg(&cmd);
> + if (!arg)
> + arg = "";
This is being cute and needs reading of the post-context a bit.
arg = next_arg(&cmd);
+ if (!arg)
+ arg = "";
if (!strcmp("OK", arg))
resp = DRV_OK;
else {
if (!strcmp("NO", arg))
resp = RESP_NO;
else /*if (!strcmp("BAD", arg))*/
resp = RESP_BAD;
fprintf(stderr, "IMAP command '%s' returned response (%s) - %s\n",
!starts_with(cmdp->cmd, "LOGIN") ?
cmdp->cmd : "LOGIN <user> <pass>",
arg, cmd ? cmd : "");
}
if ((resp2 = parse_response_code(ctx, &cmdp->cb, cmd)) > resp)
resp = resp2;
Because the existing code does not explicitly check for "BAD", we
get RESP_BAD, which is what we want in the end, and the error
message we give has "returned response (%s)" which is filled with
this empty string.
I notice that when this change matters, i.e. we hit a premature end,
next_arg() that yielded NULL would have cleared cmd. That is OK for
the fprintf() we see above, but wouldn't it hit parse_response_code()
that is shared between good and bad cases? The function starts like
so:
static int parse_response_code(struct imap_store *ctx, struct imap_cmd_cb *cb,
char *s)
{
struct imap *imap = ctx->imap;
char *arg, *p;
if (*s != '[')
return RESP_OK; /* no response code */
so we will get an immediate NULL dereference, it appears.
The fixes in other hunks looked OK (and not cute).
