count single \r or \n as \r\n while checking line length against MAXHEADERSIZE

Florian Schlichting fschlich at CIS.FU-Berlin.DE
Tue May 25 10:44:38 UTC 2010


Hi,

this is an issue I happened upon while transferring a bunch of old
articles from inn2.3 to inn2.5, but I think it might be interesting more
generally as long as as older versions of inn are still in use. 

It appears that some people like to grace their articles with Face: or
X-Face: headers of typically 4-6k binary data. While usually put into
nice continuation lines of around 80 characters, a few clients fail to
create proper line breaks for this header, inserting only a single \r or
\n instead. The nnrpd in inn2.3 doesn't reject or correct these mistakes
for non-system headers, and offending articles are still "in the wild".

innd currently counts, logs and ignores such imperfect line breaks.
Alas, when checking for the maximum allowable 'physical' line length,
single \r or \n are not recognized, thus making the whole 'virtual'
(continued) header appear as a single line, easily exceeding
MAXHEADERSIZE and thus causing the article to be rejected.

This patch causes both CRwithoutLF (\r) and LFwithoutCR (\n) to be
considered the same as CRLF (\r\n) for purposes of checking allowable
header line length. While I have been running with this patch for
several months without problems, I wonder if there could be another
place where imperfectly broken virtual lines might cause a problem?


NB: I believe the only place where MAXHEADERSIZE describes allowable
line length is here in ARTparseheader, so I think it could perhaps be
replaced by a check against 1000 in order to achieve the fix mentioned
in include/inn/options.h:118?


Florian


--- a/innd/art.c
+++ b/innd/art.c
@@ -806,8 +806,10 @@ ARTparseheader(CHANNEL *cp)
     for (i = cp->Next; i < bp->used; i++) {
         if (bp->data[i] == '\0')
             ARTerror(cp, "Nul character in header");
-        if (bp->data[i] == '\n')
+        if (bp->data[i] == '\n') {
             data->LFwithoutCR++;
+            data->LastCRLF = i;
+	}
         if (bp->data[i] != '\r')
             continue;
 
@@ -874,6 +876,7 @@ ARTparseheader(CHANNEL *cp)
             data->LastCRLF = i - 1;
         } else {
             data->CRwithoutLF++;
+            data->LastCRLF = i;
         }
     }
     cp->Next = i;
@@ -2559,7 +2562,7 @@ ARTpost(CHANNEL *cp)
                data->LFwithoutCR);
     else
       snprintf(cp->Error, sizeof(cp->Error),
-               "Article accepted but includes CR without LF(%d) and LF withtout CR(%d)",
+               "Article accepted but includes CR without LF(%d) and LF without CR(%d)",
                data->CRwithoutLF, data->LFwithoutCR);
     /* We have another ARTlog() for the same article just after. */
     ARTlog(data, ART_STRSTR, cp->Error);




More information about the inn-workers mailing list