base64 functions with SASL in INN

[Julien ÉLIE]

Hi,

    http://www.kb.cert.org/vuls/id/238019
    http://www.kb.cert.org/vuls/id/RGII-7RYLZQ

Cyrus SASL added a null character to the end of the output of sasl_encode64().

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).

I note that if a string has a length of n, the maximum length of its base64
conversion including the null terminator is (n/3+1)*4+1 so:

(n/3+1)*4+1 <= 2n+k
is equivalent to
2x >= 15-3k
which is true for all x if and only if k>=5.

I therefore believe "+6" is enough with the NULL character.  But well,
we can keep "+10".


We also need that fix in nnrpd:

Index: innfeed/imap_connection.c
===================================================================
--- innfeed/imap_connection.c   (révision 8509)
+++ innfeed/imap_connection.c   (copie de travail)
@@ -2425,7 +2425,7 @@

     /* convert to base64 */
     saslresult = sasl_encode64(out, outlen,
-                              inbase64, outlen*2, (unsigned *) &inbase64len);
+                              inbase64, outlen*2 + 10, (unsigned *) &inbase64len);

     if (saslresult != SASL_OK) return RET_FAIL;

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;
        }




If I understand well, only sasl_encode64() calls need to have "+1".
sasl_decode64() are not affected by the change.

Is it fine?

-- 
Julien ÉLIE

« L'homme a imaginé le cercle avant de savoir que la terre était ronde.
  Ça prouve quand même une certaine faculté d'invention. » (Jacques Sternberg)