INN commit: branches/2.5 (3 files)

INN Commit rra at isc.org
Sat Jun 20 13:13:24 UTC 2009


    Date: Saturday, June 20, 2009 @ 06:13:23
  Author: iulius
Revision: 8523

* Fixed a segfault in imap_connection which can occur when SASL
is used (overflow in strlcpy).

* Owing to the US-CERT vulnerability note VU#238019, Cyrus SASL library
has slightly changed. imap_connection and nnrpd now handle
that change.  Otherwise, some answers are too long to be properly
computed during SASL exchanges (because of a new null character
added by sasl_encode64).

Modified:
  branches/2.5/doc/pod/news.pod
  branches/2.5/innfeed/imap_connection.c
  branches/2.5/nnrpd/sasl.c

---------------------------+
 doc/pod/news.pod          |   12 ++++++
 innfeed/imap_connection.c |   81 ++++++++++++++++++++++++++------------------
 nnrpd/sasl.c              |    9 +++-
 3 files changed, 66 insertions(+), 36 deletions(-)

Modified: doc/pod/news.pod
===================================================================
--- doc/pod/news.pod	2009-06-20 13:09:14 UTC (rev 8522)
+++ doc/pod/news.pod	2009-06-20 13:13:23 UTC (rev 8523)
@@ -4,6 +4,18 @@
 
 =item *
 
+Fixed a segfault in B<imap_connection> which could occur when SASL
+was used.
+
+=item *
+
+Owing to the US-CERT vulnerability note VU#238019, Cyrus SASL library
+has slightly changed.  B<imap_connection> and B<nnrpd> now handle
+that change.  Otherwise, some answers are too long to be properly
+computed during SASL exchanges.
+
+=item *
+
 Fixed a memory allocation problem which caused B<nnrpd> to die when
 retrieving via HDR/XHDR/XPAT the contents of an extra overview field
 absent from the headers of an article.  The NEWNEWS command was also

Modified: innfeed/imap_connection.c
===================================================================
--- innfeed/imap_connection.c	2009-06-20 13:09:14 UTC (rev 8522)
+++ innfeed/imap_connection.c	2009-06-20 13:13:23 UTC (rev 8523)
@@ -2153,13 +2153,18 @@
 	/* empty initial client response */
         p = concat("AUTH ", mechusing, " =\r\n", (char *) 0);
     } else {
-	/* initial client response - convert to base64 */
-	inbase64 = xmalloc(outlen*2+10);
+        /* Initial client response - convert to base64.
+         * 2n+7 bytes are enough to contain the result of the base64
+         * encoding of a string whose length is n bytes.
+         * In sasl_encode64() calls, the fourth argument is the length
+         * of the third including the null terminator (thus 2n+8 bytes). */
+        inbase64 = xmalloc(outlen*2 + 8);
 
-	saslresult = sasl_encode64(out, outlen,
-				   inbase64, outlen*2+10,
+        saslresult = sasl_encode64(out, outlen,
+                                   inbase64, outlen*2 + 8,
                                    (unsigned *) &inbase64len);
-	if (saslresult != SASL_OK) return RET_FAIL;
+        if (saslresult != SASL_OK)
+            return RET_FAIL;
         p = concat("AUTH ", mechusing, " ", inbase64, "\r\n", (char *) 0);
         free(inbase64);
     }
@@ -2420,21 +2425,26 @@
 	cxn->imap_state = IMAP_CONNECTED_NOTAUTH;
 	return RET_FAIL;
     }
+    /* Convert to base64.
+     * 2n+7 bytes are enough to contain the result of the base64
+     * encoding of a string whose length is n bytes.
+     * In sasl_encode64() calls, the fourth argument is the length
+     * of the third including the null terminator (thus 2n+8 bytes).
+     * And CRLF takes the last two bytes (thus 2n+10 bytes). */
+    inbase64 = xmalloc(outlen*2 + 10);
 
-    inbase64 = xmalloc(outlen * 2 + 10);
-
-    /* convert to base64 */
     saslresult = sasl_encode64(out, outlen,
-			       inbase64, outlen*2, (unsigned *) &inbase64len);
+                               inbase64, outlen*2 + 8, (unsigned *) &inbase64len);
 
-    if (saslresult != SASL_OK) return RET_FAIL;
+    if (saslresult != SASL_OK)
+        return RET_FAIL;
 
-    /* append endline */
-    strlcpy(inbase64 + inbase64len, "\r\n", outlen * 2 + 10 - inbase64len);
-    inbase64len+=2;
+    /* Append endline. */
+    strlcpy(inbase64 + inbase64len, "\r\n", outlen*2 + 10 - inbase64len);
+    inbase64len += 2;
     
-    /* send to server */
-    result = WriteToWire_imapstr(cxn,inbase64, inbase64len);
+    /* Send to server. */
+    result = WriteToWire_imapstr(cxn, inbase64, inbase64len);
     
     cxn->imap_state = IMAP_WRITING_STEPAUTH;
 
@@ -3386,29 +3396,34 @@
 			return;
 		    }
 
-		    /* convert to base64 */
-		    inbase64 = xmalloc(outlen*2+10);
+                    /* Convert to base64.
+                     * 2n+7 bytes are enough to contain the result of the base64
+                     * encoding of a string whose length is n bytes.
+                     * In sasl_encode64() calls, the fourth argument is the length
+                     * of the third including the null terminator (thus 2n+8 bytes).
+                     * And CRLF takes the last two bytes (thus 2n+10 bytes). */
+                    inbase64 = xmalloc(outlen*2 + 10);
 
-		    saslresult = sasl_encode64(out, outlen,
-					       inbase64, outlen*2+10, 
-					       (unsigned *) &inbase64len);
-		    
-		    if (saslresult != SASL_OK)
-		    {
-			d_printf(0,"%s:%d:LMTP sasl_encode64(): %s\n",
-				 hostPeerName (cxn->myHost),cxn->ident,
-				 sasl_errstring(saslresult,NULL,NULL));
+                    saslresult = sasl_encode64(out, outlen,
+                                               inbase64, outlen*2 + 8,
+                                               (unsigned *) &inbase64len);
 
-			lmtp_Disconnect(cxn);
-			return;
-		    }
+                    if (saslresult != SASL_OK) {
+                        d_printf(0,"%s:%d:LMTP sasl_encode64(): %s\n",
+                                 hostPeerName(cxn->myHost), cxn->ident,
+                                 sasl_errstring(saslresult, NULL, NULL));
 
-		    /* add an endline */
-		    strlcpy(inbase64 + inbase64len, "\r\n", outlen * 2 + 10);
+                        lmtp_Disconnect(cxn);
+                        return;
+                    }
 
-		    /* send to server */
-		    result = WriteToWire_lmtpstr(cxn,inbase64, inbase64len+2);
+                    /* Add an endline. */
+                    strlcpy(inbase64 + inbase64len, "\r\n", outlen*2 + 10 - inbase64len);
+                    inbase64len += 2;
 
+                    /* Send to server. */
+                    result = WriteToWire_lmtpstr(cxn, inbase64, inbase64len);
+
 		    if (result != RET_OK)
 		    {
 			d_printf(0,"%s:%d:LMTP WriteToWire() failure\n",

Modified: nnrpd/sasl.c
===================================================================
--- nnrpd/sasl.c	2009-06-20 13:09:14 UTC (rev 8522)
+++ nnrpd/sasl.c	2009-06-20 13:13:23 UTC (rev 8523)
@@ -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:  (floor(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;
 	}




More information about the inn-committers mailing list