INN commit: branches/2.6 (10 files)

INN Commit rra at isc.org
Sat Sep 17 20:40:43 UTC 2016


    Date: Saturday, September 17, 2016 @ 13:40:43
  Author: iulius
Revision: 10071

nnrpd:  add syntax checks at injection time for header fields

Control chars and empty content lines in header fields are now
rejected by nnrpd at injection time.

Add two new functions IsValidHeaderBody() and IsValidHeaderField()
in libinn, along with a test suite.

Added:
  branches/2.6/tests/lib/headers-t.c
    (from rev 10070, trunk/tests/lib/headers-t.c)
Modified:
  branches/2.6/MANIFEST
  branches/2.6/doc/pod/news.pod
  branches/2.6/include/inn/libinn.h
  branches/2.6/lib/headers.c
  branches/2.6/nnrpd/post.c
  branches/2.6/support/mkmanifest
  branches/2.6/tests/Makefile
  branches/2.6/tests/TESTS
  branches/2.6/tests/lib/	(properties)

-----------------------+
 MANIFEST              |    1 
 doc/pod/news.pod      |    9 +++-
 include/inn/libinn.h  |    2 
 lib/headers.c         |  103 ++++++++++++++++++++++++++++++++++++++++++------
 nnrpd/post.c          |   32 ++++++++++++--
 support/mkmanifest    |    1 
 tests/Makefile        |    5 +-
 tests/TESTS           |    1 
 tests/lib             |    1 
 tests/lib/headers-t.c |   70 ++++++++++++++++++++++++++++++++
 10 files changed, 204 insertions(+), 21 deletions(-)

Modified: MANIFEST
===================================================================
--- MANIFEST	2016-09-17 20:37:15 UTC (rev 10070)
+++ MANIFEST	2016-09-17 20:40:43 UTC (rev 10071)
@@ -889,6 +889,7 @@
 tests/lib/getnameinfo-t.c             Tests for lib/getnameinfo.c
 tests/lib/hash-t.c                    Tests for lib/hash.c
 tests/lib/hashtab-t.c                 Tests for lib/hashtab.c
+tests/lib/headers-t.c                 Tests for lib/headers.c
 tests/lib/hex-t.c                     Tests for lib/hex.c
 tests/lib/inet_aton-t.c               Tests for lib/inet_aton.c
 tests/lib/inet_ntoa-t.c               Tests for lib/inet_ntoa.c

Modified: doc/pod/news.pod
===================================================================
--- doc/pod/news.pod	2016-09-17 20:37:15 UTC (rev 10070)
+++ doc/pod/news.pod	2016-09-17 20:40:43 UTC (rev 10071)
@@ -39,11 +39,16 @@
 
 =item *
 
-If an article contains a Received: or a Posted: header field, it is no
-longer rejected by B<nnrpd>.
+Articles containing a Received: or a Posted: header field are no longer
+rejected by B<nnrpd> at injection time.
 
 =item *
 
+Articles containing control characters or whitespace-only content lines
+in their headers are now rejected by B<nnrpd> at injection time.
+
+=item *
+
 S<OpenSSL 1.1.0> support has been added to INN.
 
 =item *

Modified: include/inn/libinn.h
===================================================================
--- include/inn/libinn.h	2016-09-17 20:37:15 UTC (rev 10070)
+++ include/inn/libinn.h	2016-09-17 20:40:43 UTC (rev 10071)
@@ -93,6 +93,8 @@
 extern void             InitializeMessageIDcclass(void);
 extern bool             IsValidMessageID(const char *string, bool stripspaces);
 extern bool             IsValidHeaderName(const char *string);
+extern bool             IsValidHeaderBody(const char *string);
+extern bool             IsValidHeaderField(const char *string);
 extern const char *     skip_cfws(const char *p);
 extern const char *     skip_fws(const char *p);
 extern void             HeaderCleanFrom(char *from);

Modified: lib/headers.c
===================================================================
--- lib/headers.c	2016-09-17 20:37:15 UTC (rev 10070)
+++ lib/headers.c	2016-09-17 20:40:43 UTC (rev 10071)
@@ -11,26 +11,21 @@
 
 
 /*
-**  We currently only check the requirements for RFC 3977:
+**  Check whether the argument is a valid header field name.
 **
+**  We currently assume the maximal line length has already been checked.
+**  Only ensure the requirements for RFC 3977:
+**
 **    o  The name [of a header] consists of one or more printable
 **       US-ASCII characters other than colon.
 */
 bool
-IsValidHeaderName(const char *string)
+IsValidHeaderName(const char *p)
 {
-    const unsigned char *p;
-
-    /* Not NULL. */
-    if (string == NULL)
+    /* Not NULL and not empty. */
+    if (p == NULL || *p == '\0')
         return false;
 
-    p = (const unsigned char *) string;
-   
-    /* Not empty. */
-    if (*p == '\0')
-        return false;
-
     for (; *p != '\0'; p++) {
         /* Contains only printable US-ASCII characters other
          * than colon. */
@@ -43,6 +38,90 @@
 
 
 /*
+**  Check whether the argument is a valid header field body.  It starts
+**  after the space following the header field name and its colon.
+**
+**  We currently assume the maximal line length has already been checked.
+*/
+bool
+IsValidHeaderBody(const char *p)
+{
+    bool emptycontentline = true;
+
+    /* Not NULL and not empty. */
+    if (p == NULL || *p == '\0')
+        return false;
+
+    for (; *p != '\0'; p++) {
+        if (isgraph((unsigned char) *p)) {
+            /* Current header content line contains a (non-whitespace)
+             * printable char. */
+            emptycontentline = false;
+            continue;
+        } else if (ISWHITE(*p)) {
+            /* Skip SP and TAB. */
+            continue;
+        } else if (*p == '\n' || (*p == '\r' && *++p == '\n')) {
+            /* Folding detected.  We expect CRLF or lone LF as some parts
+             * of INN code internally remove CR.
+             * Check that the line that has just been processed is not
+             * "empty" and that the following character marks the beginning
+             * of a continuation line. */
+            if (emptycontentline || !ISWHITE(p[1])) {
+                return false;
+            }
+            /* A continuation line begins.  This new line should also have
+             * at least one printable octet other than SP or TAB, so we
+             * re-initialize emptycontentline to true. */
+            emptycontentline = true;
+            continue;
+        } else {
+            /* Invalid character found. */
+            return false;
+        }
+    }
+
+    return (!emptycontentline);
+}
+
+
+/*
+**  Check whether the argument is a valid header field.
+**
+**  We currently assume the maximal line length has already been checked.
+*/
+bool
+IsValidHeaderField(const char *p)
+{
+    /* Not NULL, not empty, and does not begin with a colon. */
+    if (p == NULL || *p == '\0' || *p == ':')
+        return false;
+
+    for (; *p != '\0'; p++) {
+        /* Header field names contain only printable US-ASCII characters
+         * other than colon.  A colon terminates the header field name. */
+        if (!isgraph((unsigned char) *p))
+            return false;
+        if (*p == ':') {
+            p++;
+            break;
+        }
+    }
+
+    /* Empty body or no colon found in header field. */
+    if (*p == '\0')
+        return false;
+
+    /* Missing space after colon. */
+    if (*p != ' ')
+        return false;
+
+    p++;
+    return IsValidHeaderBody(p);
+}
+
+
+/*
 **  Skip any amount of CFWS (comments and folding whitespace), the RFC 5322
 **  grammar term for whitespace, CRLF pairs, and possibly nested comments that
 **  may contain escaped parens.  We also allow simple newlines since we don't

Modified: nnrpd/post.c
===================================================================
--- nnrpd/post.c	2016-09-17 20:37:15 UTC (rev 10070)
+++ nnrpd/post.c	2016-09-17 20:40:43 UTC (rev 10071)
@@ -349,6 +349,7 @@
     const char          *error;
     pid_t               pid;
     bool		addvirtual = false;
+    int                 i;
 
     /* Get the current time, used for creating and checking dates. */
     now = time(NULL);
@@ -367,15 +368,22 @@
 		     "Can't set system %s: header", hp->Name);
 	    return Error;
 	}
-	if (hp->Value) {
-	    hp->Len = TrimSpaces(hp->Value);
+        if (hp->Value) {
+            hp->Len = TrimSpaces(hp->Value);
             /* If the header is empty, we just remove it.  We do not reject
              * the article, contrary to what an injecting agent is supposed
              * to do per Section 3.5 of RFC 5537.  (A revision to RFC 5537
              * may someday allow again that existing and useful feature.) */
-	    if (hp->Len == 0)
-		hp->Value = hp->Body = NULL;
-	}
+            if (hp->Len == 0) {
+                hp->Value = hp->Body = NULL;
+            } else if (!IsValidHeaderBody(hp->Value)) {
+                snprintf(Error, sizeof(Error),
+                         "Invalid syntax encountered in %s: header field "
+                         "body (unexpected byte or empty content line)",
+                         hp->Name);
+                return Error;
+            }
+        }
     }
 
     /* Set the Injection-Date: header. */
@@ -625,6 +633,16 @@
 	    return Error;
 	}
 
+    /* Check that all other header fields are valid. */
+    for (i = 0; i < OtherCount; i++) {
+        if (!IsValidHeaderField(OtherHeaders[i])) {
+            snprintf(Error, sizeof(Error),
+                     "Invalid syntax encountered in headers (unexpected "
+                     "byte, no colon-space, or empty content line)");
+            return Error;
+        }
+    }
+
     return NULL;
 }
 
@@ -1160,7 +1178,9 @@
     }
 
 #if defined(DO_PERL)
-    /* Calls the Perl subroutine for headers management. */
+    /* Calls the Perl subroutine for headers management.
+     * The article may be modified, and its syntax may become invalid
+     * but well... that's the news admin choice! */
     p = PERMaccessconf->nnrpdperlfilter ? HandleHeaders(article) : NULL;
     if (p != NULL) {
         char SDir[255];

Modified: support/mkmanifest
===================================================================
--- support/mkmanifest	2016-09-17 20:37:15 UTC (rev 10070)
+++ support/mkmanifest	2016-09-17 20:40:43 UTC (rev 10071)
@@ -279,6 +279,7 @@
 tests/lib/getnameinfo.t
 tests/lib/hash.t
 tests/lib/hashtab.t
+tests/lib/headers.t
 tests/lib/hex.t
 tests/lib/inet_aton.t
 tests/lib/inet_ntoa.t

Modified: tests/Makefile
===================================================================
--- tests/Makefile	2016-09-17 20:37:15 UTC (rev 10070)
+++ tests/Makefile	2016-09-17 20:40:43 UTC (rev 10071)
@@ -19,7 +19,7 @@
 	lib/buffer.t lib/concat.t lib/conffile.t lib/confparse.t lib/date.t \
 	lib/dispatch.t lib/fdflag.t \
 	lib/getaddrinfo.t lib/getnameinfo.t lib/hash.t \
-	lib/hashtab.t lib/hex.t lib/inet_aton.t \
+	lib/hashtab.t lib/headers.t lib/hex.t lib/inet_aton.t \
 	lib/inet_ntoa.t lib/inet_ntop.t lib/innconf.t lib/list.t lib/md5.t \
 	lib/memcmp.t lib/messages.t lib/mkstemp.t \
 	lib/network/addr-ipv4.t lib/network/addr-ipv6.t \
@@ -133,6 +133,9 @@
 lib/hashtab.t: lib/hashtab-t.o tap/basic.o $(LIBINN)
 	$(LINK) lib/hashtab-t.o tap/basic.o $(LIBINN)
 
+lib/headers.t: lib/headers-t.o tap/basic.o $(LIBINN)
+	$(LINK) lib/headers-t.o tap/basic.o $(LIBINN)
+
 lib/hex.t: lib/hex-t.o tap/basic.o $(LIBINN)
 	$(LINK) lib/hex-t.o tap/basic.o $(LIBINN) $(LIBS)
 

Modified: tests/TESTS
===================================================================
--- tests/TESTS	2016-09-17 20:37:15 UTC (rev 10070)
+++ tests/TESTS	2016-09-17 20:40:43 UTC (rev 10071)
@@ -17,6 +17,7 @@
 lib/getnameinfo
 lib/hash
 lib/hashtab
+lib/headers
 lib/hex
 lib/inet_aton
 lib/inet_ntoa

Index: branches/2.6/tests/lib
===================================================================
--- tests/lib	2016-09-17 20:37:15 UTC (rev 10070)
+++ tests/lib	2016-09-17 20:40:43 UTC (rev 10071)

Property changes on: branches/2.6/tests/lib
___________________________________________________________________
Modified: svn:ignore
## -12,6 +12,7 ##
 getnameinfo.t
 hash.t
 hashtab.t
+headers.t
 hex.t
 hstrerror.t
 inet_aton.t
Copied: branches/2.6/tests/lib/headers-t.c (from rev 10070, trunk/tests/lib/headers-t.c)
===================================================================
--- tests/lib/headers-t.c	                        (rev 0)
+++ tests/lib/headers-t.c	2016-09-17 20:40:43 UTC (rev 10071)
@@ -0,0 +1,70 @@
+/*
+** headers test suite.
+**
+** $Id$
+*/
+
+#define LIBTEST_NEW_FORMAT 1
+
+#include "inn/libinn.h"
+#include "tap/basic.h"
+
+int
+main(void)
+{
+    plan(9+3+9+7+12+5);
+
+    ok(!IsValidHeaderName(NULL), "bad header name 1");
+    ok(!IsValidHeaderName(""), "bad header name 2");
+    ok(!IsValidHeaderName(":"), "bad header name 3");
+    ok(!IsValidHeaderName("Sub:ject"), "bad header name 4");
+    ok(!IsValidHeaderName("Subject:"), "bad header name 5");
+    /* \177 (octal notation) is DEL. */
+    ok(!IsValidHeaderName("\177Subject"), "bad header name 6");
+    ok(!IsValidHeaderName("Sub ject"), "bad header name 7");
+    ok(!IsValidHeaderName("Sub\tject"), "bad header name 8");
+    ok(!IsValidHeaderName("Sub\r\nject"), "bad header name 9");
+    
+    ok(IsValidHeaderName("Subject"), "good header name 1");
+    ok(IsValidHeaderName("subJECT"), "good header name 2");
+    ok(IsValidHeaderName("X-#%-T`?!"), "good header name 3");
+
+    ok(!IsValidHeaderBody(NULL), "bad header body 1");
+    ok(!IsValidHeaderBody(""), "bad header body 2");
+    ok(!IsValidHeaderBody("a\177b"), "bad header body 3");
+    ok(!IsValidHeaderBody("a\r\nb"), "bad header body 4");
+    ok(!IsValidHeaderBody("a\nb"), "bad header body 5");
+    ok(!IsValidHeaderBody("\n"), "bad header body 6");
+    ok(!IsValidHeaderBody("\r\n b"), "bad header body 7");
+    ok(!IsValidHeaderBody("a\r\n b\r\n"), "bad header body 8");
+    ok(!IsValidHeaderBody("a\n\tb\n \t\n c"), "bad header body 9");
+
+    ok(IsValidHeaderBody(":"), "good header body 1");
+    ok(IsValidHeaderBody("a b"), "good header body 2");
+    ok(IsValidHeaderBody("a\t\tb"), "good header body 3");
+    ok(IsValidHeaderBody("a\r\n b"), "good header body 4");
+    ok(IsValidHeaderBody("a\r\n\tb"), "good header body 5");
+    ok(IsValidHeaderBody("a\n   b"), "good header body 6");
+    ok(IsValidHeaderBody("a\n\tb\n \tc\n d"), "good header body 7");
+
+    ok(!IsValidHeaderField(NULL), "bad header field 1");
+    ok(!IsValidHeaderField(""), "bad header field 2");
+    ok(!IsValidHeaderField(":"), "bad header field 3");
+    ok(!IsValidHeaderField("Subject"), "bad header field 4");
+    ok(!IsValidHeaderField("Subject:"), "bad header field 5");
+    ok(!IsValidHeaderField("Sub:ject"), "bad header field 6");
+    ok(!IsValidHeaderField("Subject: "), "bad header field 7");
+    ok(!IsValidHeaderField("Subject:\ta"), "bad header field 8");
+    ok(!IsValidHeaderField("Subject: a\n"), "bad header field 9");
+    ok(!IsValidHeaderField("\177Subject: a"), "bad header field 10");
+    ok(!IsValidHeaderField("Subject: a\177b"), "bad header field 11");
+    ok(!IsValidHeaderField("Subject: a\nb"), "bad header field 12");
+
+    ok(IsValidHeaderField("Subject: a"), "good header field 1");
+    ok(IsValidHeaderField("Subject: a\n\tb"), "good header field 2");
+    ok(IsValidHeaderField("Sub: ject"), "good header field 3");
+    ok(IsValidHeaderField("X-#%-T`?!: yeah"), "good header field 4");
+    ok(IsValidHeaderField("Subject: a\r\n\tb"), "good header field 5");
+
+    return 0;
+}



More information about the inn-committers mailing list