INN commit: trunk (10 files)
INN Commit
rra at isc.org
Sat Sep 17 20:37:16 UTC 2016
Date: Saturday, September 17, 2016 @ 13:37:15
Author: iulius
Revision: 10070
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:
trunk/tests/lib/headers-t.c
Modified:
trunk/MANIFEST
trunk/doc/pod/news.pod
trunk/include/inn/libinn.h
trunk/lib/headers.c
trunk/nnrpd/post.c
trunk/support/mkmanifest
trunk/tests/Makefile
trunk/tests/TESTS
trunk/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-04 13:15:36 UTC (rev 10069)
+++ MANIFEST 2016-09-17 20:37:15 UTC (rev 10070)
@@ -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-04 13:15:36 UTC (rev 10069)
+++ doc/pod/news.pod 2016-09-17 20:37:15 UTC (rev 10070)
@@ -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-04 13:15:36 UTC (rev 10069)
+++ include/inn/libinn.h 2016-09-17 20:37:15 UTC (rev 10070)
@@ -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-04 13:15:36 UTC (rev 10069)
+++ lib/headers.c 2016-09-17 20:37:15 UTC (rev 10070)
@@ -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-04 13:15:36 UTC (rev 10069)
+++ nnrpd/post.c 2016-09-17 20:37:15 UTC (rev 10070)
@@ -349,6 +349,7 @@
const char *error;
pid_t pid;
bool addvirtual = false;
+ size_t 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;
}
@@ -1165,7 +1183,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-04 13:15:36 UTC (rev 10069)
+++ support/mkmanifest 2016-09-17 20:37:15 UTC (rev 10070)
@@ -281,6 +281,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-04 13:15:36 UTC (rev 10069)
+++ tests/Makefile 2016-09-17 20:37:15 UTC (rev 10070)
@@ -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-04 13:15:36 UTC (rev 10069)
+++ tests/TESTS 2016-09-17 20:37:15 UTC (rev 10070)
@@ -17,6 +17,7 @@
lib/getnameinfo
lib/hash
lib/hashtab
+lib/headers
lib/hex
lib/inet_aton
lib/inet_ntoa
Index: trunk/tests/lib
===================================================================
--- tests/lib 2016-09-04 13:15:36 UTC (rev 10069)
+++ tests/lib 2016-09-17 20:37:15 UTC (rev 10070)
Property changes on: trunk/tests/lib
___________________________________________________________________
Modified: svn:ignore
## -12,6 +12,7 ##
getnameinfo.t
hash.t
hashtab.t
+headers.t
hex.t
hstrerror.t
inet_aton.t
Added: tests/lib/headers-t.c
===================================================================
--- tests/lib/headers-t.c (rev 0)
+++ tests/lib/headers-t.c 2016-09-17 20:37:15 UTC (rev 10070)
@@ -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;
+}
Property changes on: trunk/tests/lib/headers-t.c
___________________________________________________________________
Added: svn:keywords
## -0,0 +1 ##
+Author Date Id Revision
\ No newline at end of property
Added: svn:eol-style
## -0,0 +1 ##
+native
\ No newline at end of property
More information about the inn-committers
mailing list