base64 functions with SASL in INN

[Russ Allbery]

Julien ÉLIE <julien at trigofacile.com> writes:

> According to <https://bugzilla.redhat.com/show_bug.cgi?id=501452>, we need
> in imap_connection.c:
>
>     inbase64 = xmalloc(outlen * 2 + 10);
>
>     /* convert to base64 */
>     saslresult = sasl_encode64(out, outlen,
> -          inbase64, outlen*2, (unsigned *) &inbase64len);
> +          inbase64, outlen*2 + 1, (unsigned *) &inbase64len);
>
>     if (saslresult != SASL_OK) return RET_FAIL;
>
>    /* append endline */
>    strlcpy(inbase64 + inbase64len, "\r\n", outlen * 2 + 10 - inbase64len);
>    inbase64len+=2;
>
> But I believe it should be "outlen*2 + 10" (and thus, there was
> already a bug).

It should at most be +8, since we want to have the space to append the
CRLF into the same buffer.

> Index: nnrpd/sasl.c
> ===================================================================
> --- nnrpd/sasl.c        (révision 8509)
> +++ nnrpd/sasl.c        (copie de travail)
> @@ -32,7 +32,8 @@
>     { SASL_CB_LIST_END, NULL, NULL }
> };
>
> -#define BASE64_BUF_SIZE 21848  /* Per RFC 2222bis:  ((16K / 3) + 1) * 4. */
> +#define BASE64_BUF_SIZE 21848  /* Per RFC 4422:  (E(n/3) + 1) * 4
> +                                   where n = 16 kB = 16384 bytes. */
>
>
> /*
> @@ -189,9 +190,11 @@
>
>     while (r == SASL_CONTINUE || (r == SASL_OK && serveroutlen != 0)) {
>        if (serveroutlen != 0) {
> -           /* Encode the server challenge. */
> +           /* Encode the server challenge.
> +             * In sasl_encode64() calls, the fourth argument is the length
> +             * of the third including the null terminator. */
>             r1 = sasl_encode64(serverout, serveroutlen,
> -                               base64, BASE64_BUF_SIZE, NULL);
> +                               base64, BASE64_BUF_SIZE+1, NULL);
>             if (r1 != SASL_OK)
>                 r = r1;
>        }

This change looks wrong -- it looks like we're telling sasl_encode64
that we have a larger buffer than we actually do.  That could lead it to
writing beyond the end of the buffer.  I think that argument should be
whatever the buffer size really is.

-- 
Russ Allbery (rra at stanford.edu)             <http://www.eyrie.org/~eagle/>

    Please send questions to the list rather than mailing me directly.
     <http://www.eyrie.org/~eagle/faqs/questions.html> explains why.