INN commit: trunk (4 files)

INN Commit rra at isc.org
Wed Oct 21 17:38:42 UTC 2009


    Date: Wednesday, October 21, 2009 @ 10:38:42
  Author: iulius
Revision: 8674

Add support for whitespaces in usernames/passwords provided
with AUTHINFO USER/PASS.  nnrpd was previously considering
as invalid passwords like "a b" (two arguments "a" and "b"
when only one was expected) or parsing " ab" as "ab"
(stripping the leading space).

nnrpd now treats everything after the first whitespace
character following AUTHINFO USER/PASS, up to, but not
including, the CRLF, as the username/password.

Thanks to Jeffrey M. Vinocur for the initial patch.

close #30

Modified:
  trunk/include/inn/libinn.h
  trunk/lib/argparse.c
  trunk/nnrpd/commands.c
  trunk/nnrpd/nnrpd.c

----------------------+
 include/inn/libinn.h |    2 
 lib/argparse.c       |   79 ++++++++++++++++++++++++++++++++++----
 nnrpd/commands.c     |   17 +++-----
 nnrpd/nnrpd.c        |  100 ++++++++++++++++++++++++++++---------------------
 4 files changed, 139 insertions(+), 59 deletions(-)

Modified: include/inn/libinn.h
===================================================================
--- include/inn/libinn.h	2009-10-20 19:36:22 UTC (rev 8673)
+++ include/inn/libinn.h	2009-10-21 17:38:42 UTC (rev 8674)
@@ -93,6 +93,8 @@
 **  TIME AND DATE PARSING, GENERATION, AND HANDLING
 */
 extern int      Argify(char *line, char ***argvp);
+extern int      nArgify(char *line, char ***argvp, int n);
+extern int      reArgify(char *p, char **argv, int n, bool stripspaces);
 extern char *   Glom(char **av);
 extern bool     makedate(time_t, bool local, char *buff, size_t buflen);
 extern time_t   parsedate_nntp(const char *, const char *, bool local);

Modified: lib/argparse.c
===================================================================
--- lib/argparse.c	2009-10-20 19:36:22 UTC (rev 8673)
+++ lib/argparse.c	2009-10-21 17:38:42 UTC (rev 8674)
@@ -11,6 +11,7 @@
 #include "inn/innconf.h"
 #include "inn/libinn.h"
 
+
 /*
 **  Parse a string into a NULL-terminated array of words; return number
 **  of words.  If argvp isn't NULL, it and what it points to will be freed.
@@ -18,7 +19,21 @@
 int
 Argify(char *line, char ***argvp)
 {
-    char        **argv;
+    return nArgify(line, argvp, -1);
+}
+
+
+/*
+**  Parse a string into a NULL-terminated array of at most n words;
+**  return number of words.  If there are more than n words, stop
+**  processing at the beginning of the (n+1)th word, store everything
+**  from the beginning of word (n+1) in argv[n] and return (n+1).
+**  If n is negative, parses all words.  If argvp isn't NULL, it and
+**  what it points to will be freed.
+*/
+int
+nArgify(char *line, char ***argvp, int n)
+{
     char        *p;
 
     if (*argvp != NULL) {
@@ -32,19 +47,67 @@
     p = xstrdup(line);
 
     /* Allocate worst-case amount of space. */
-    for (*argvp = argv = xmalloc((strlen(p) + 2) * sizeof(char *)); *p; ) {
-        /* Mark start of this word, find its end. */
-        for (*argv++ = p; *p && !ISWHITE(*p); )
+    *argvp = xmalloc((strlen(p) + 2) * sizeof(char *));
+
+    return reArgify(p, *argvp, n, true);
+}
+
+
+/*
+**  Destructively parse a string into a NULL-terminated array of at most
+**  n words; return number of words.  Behaviour on negative n and strings
+**  of more than n words matches that of nArgify (see above).  Caller
+**  must supply an array of sufficient size (such as created by nArgify).
+**
+**  Note that the sequence
+**     ac = nArgify(line, &argv, n1);
+**     ac--;
+**     ac += reArgify(argv[ac], &argv[ac], n2, true);
+**  is equivalent to
+**     ac = nArgify(line, &argv, n1 + n2);
+**
+**  It is sometimes useful not to strip spaces.  For instance
+**  "AUTHINFO PASS  test" should be understood as giving the
+**  password " test", beginning with a space.
+*/
+int
+reArgify(char *p, char **argv, int n, bool stripspaces)
+{
+    char **save = argv;
+
+    /* Should never happen unless caller modifies argv between calls
+     * or stripspaces has previously been set to false. */
+    if (stripspaces) {
+        while (ISWHITE(*p))
             p++;
+    }
+
+    for ( ; *p; ) {
+        if (n == 0) {
+            *argv++ = p;
+            break;
+        }
+
+        /* Decrement limit, mark start of this word, find its end. */
+        for (n--, *argv++ = p; *p && !ISWHITE(*p); )
+            p++;
+
         if (*p == '\0')
             break;
 
-        /* Nip off word, skip whitespace. */
-        for (*p++ = '\0'; ISWHITE(*p); )
-            p++;
+        /* Nip off word. */
+        *p++ = '\0';
+        
+        /* Skip whitespace. */
+        if (stripspaces) {
+            while (ISWHITE(*p))
+                p++;
+        }
     }
+
     *argv = NULL;
-    return argv - *argvp;
+
+    return argv - save;
 }
 
 

Modified: nnrpd/commands.c
===================================================================
--- nnrpd/commands.c	2009-10-20 19:36:22 UTC (rev 8673)
+++ nnrpd/commands.c	2009-10-21 17:38:42 UTC (rev 8674)
@@ -252,6 +252,10 @@
     if (strcasecmp(av[1], "GENERIC") == 0) {
 	char *logrec = Glom(av);
 
+        /* Go on parsing the command line. */
+        ac--;
+        ac += reArgify(av[ac], &av[ac], -1, true);
+
 	strlcpy(PERMuser, "<none>", sizeof(PERMuser));
 
         /* Arguments are checked by PERMgeneric(). */
@@ -282,6 +286,10 @@
 	}
     } else if (strcasecmp(av[1], "SASL") == 0) {
 #ifdef HAVE_SASL
+        /* Go on parsing the command line. */
+        ac--;
+        ac += reArgify(av[ac], &av[ac], -1, true);
+
         /* Arguments are checked by SASLauth(). */
 	SASLauth(ac, av);
 #else
@@ -309,10 +317,6 @@
             }
 #endif
 
-            if (ac > 3) {
-                Reply("%d No whitespace allowed in username\r\n", NNTP_ERR_SYNTAX);
-                return;
-            }
             strlcpy(User, av[2], sizeof(User));
             Reply("%d Enter password\r\n", NNTP_CONT_AUTHINFO);
             return;
@@ -349,11 +353,6 @@
             return;
         }
 
-        if (ac > 3) {
-            Reply("%d No whitespace allowed in password\r\n", NNTP_ERR_SYNTAX);
-            return;
-        }
-
         /* There is a cached username and a password is provided. */
         strlcpy(Password, av[2], sizeof(Password));
 

Modified: nnrpd/nnrpd.c
===================================================================
--- nnrpd/nnrpd.c	2009-10-20 19:36:22 UTC (rev 8673)
+++ nnrpd/nnrpd.c	2009-10-21 17:38:42 UTC (rev 8674)
@@ -70,6 +70,7 @@
     bool                Needauth;
     int                 Minac;
     int                 Maxac;
+    bool                Stripspaces;
     const char *        Help;
 } CMDENT;
 
@@ -107,74 +108,76 @@
 
 /*
 **  { command base name, function to call, need authentication,
-**    min args, max args, help string }
+**    min args, max args, strip spaces, help string }
 */
 static CMDENT	CMDtable[] = {
-    {	"ARTICLE",	CMDfetch,	true,	1,	2,
+    {	"ARTICLE",	CMDfetch,	true,	1,	2,      true,
 	CMDfetchhelp },
-    {   "AUTHINFO",     CMDauthinfo,    false,  3,      CMDany,
+    /* Parse AUTHINFO in a special way so as to keep white spaces
+     * in usernames and passwords. */
+    {   "AUTHINFO",     CMDauthinfo,    false,  3,      CMDany, false,
         "USER name|PASS password"
 #ifdef HAVE_SASL
         "|SASL mechanism [initial-response]"
 #endif
         "|GENERIC program [argument ...]" },
-    {	"BODY",		CMDfetch,	true,	1,	2,
+    {	"BODY",		CMDfetch,	true,	1,	2,      true,
 	CMDfetchhelp },
-    {   "CAPABILITIES", CMDcapabilities,false,  1,      2,
+    {   "CAPABILITIES", CMDcapabilities,false,  1,      2,      true,
         "[keyword]" },
-    {	"DATE",		CMDdate,	false,	1,	1,
+    {	"DATE",		CMDdate,	false,	1,	1,      true,
 	NULL },
-    {	"GROUP",	CMDgroup,	true,	2,	2,
+    {	"GROUP",	CMDgroup,	true,	2,	2,      true,
 	"newsgroup" },
-    {   "HDR",          CMDpat  ,       true,   2,      3,
+    {   "HDR",          CMDpat  ,       true,   2,      3,      true,
         "header [message-ID|range]" },
-    {	"HEAD",		CMDfetch,	true,	1,	2,
+    {	"HEAD",		CMDfetch,	true,	1,	2,      true,
 	CMDfetchhelp },
-    {	"HELP",		CMDhelp,	false,	1,	1,
+    {	"HELP",		CMDhelp,	false,	1,	1,      true,
 	NULL },
-    {	"IHAVE",	CMDpost,	true,	2,	2,
+    {	"IHAVE",	CMDpost,	true,	2,	2,      true,
 	"message-ID" },
-    {	"LAST",		CMDnextlast,	true,	1,	1,
+    {	"LAST",		CMDnextlast,	true,	1,	1,      true,
 	NULL },
-    {	"LIST",		CMDlist,	true,	1,	3,
+    {	"LIST",		CMDlist,	true,	1,	3,      true,
 	"[ACTIVE [wildmat]|ACTIVE.TIMES [wildmat]|DISTRIB.PATS|DISTRIBUTIONS"
         "|HEADERS [MSGID|RANGE]|MODERATORS|MOTD|NEWSGROUPS [wildmat]"
         "|OVERVIEW.FMT|SUBSCRIPTIONS]" },
-    {	"LISTGROUP",	CMDgroup,	true,	1,	3,
+    {	"LISTGROUP",	CMDgroup,	true,	1,	3,      true,
 	"[newsgroup [range]]" },
-    {	"MODE",		CMDmode,	false,	2,	2,
+    {	"MODE",		CMDmode,	false,	2,	2,      true,
 	"READER" },
-    {	"NEWGROUPS",	CMDnewgroups,	true,	3,	4,
+    {	"NEWGROUPS",	CMDnewgroups,	true,	3,	4,      true,
 	"[yy]yymmdd hhmmss [GMT]" },
-    {	"NEWNEWS",	CMDnewnews,	true,	4,	5,
+    {	"NEWNEWS",	CMDnewnews,	true,	4,	5,      true,
 	"wildmat [yy]yymmdd hhmmss [GMT]" },
-    {	"NEXT",		CMDnextlast,	true,	1,	1,
+    {	"NEXT",		CMDnextlast,	true,	1,	1,      true,
 	NULL },
-    {   "OVER",         CMDover,        true,   1,      2,
+    {   "OVER",         CMDover,        true,   1,      2,      true,
         "[range]" },
-    {	"POST",		CMDpost,	true,	1,	1,
+    {	"POST",		CMDpost,	true,	1,	1,      true,
 	NULL },
-    {   "QUIT",         CMDquit,        false,  1,      1,
+    {   "QUIT",         CMDquit,        false,  1,      1,      true,
         NULL },
     /* SLAVE (which was ill-defined in RFC 977) was removed from the NNTP
      * protocol in RFC 3977. */
-    {	"SLAVE",	CMD_unimp,	false,	1,	1,
+    {	"SLAVE",	CMD_unimp,	false,	1,	1,      true,
 	NULL },
 #ifdef HAVE_SSL
-    {   "STARTTLS",     CMDstarttls,    false,  1,      1,
+    {   "STARTTLS",     CMDstarttls,    false,  1,      1,      true,
         NULL },
 #endif
-    {	"STAT",		CMDfetch,	true,	1,	2,
+    {	"STAT",		CMDfetch,	true,	1,	2,      true,
 	CMDfetchhelp },
-    {	"XGTITLE",	CMDxgtitle,	true,	1,	2,
+    {	"XGTITLE",	CMDxgtitle,	true,	1,	2,      true,
 	"[wildmat]" },
-    {	"XHDR",		CMDpat,		true,	2,	3,
+    {	"XHDR",		CMDpat,		true,	2,	3,      true,
 	"header [message-ID|range]" },
-    {	"XOVER",	CMDover,	true,	1,	2,
+    {	"XOVER",	CMDover,	true,	1,	2,      true,
 	"[range]" },
-    {	"XPAT",		CMDpat,		true,	4,	CMDany,
+    {	"XPAT",		CMDpat,		true,	4,	CMDany,      true,
 	"header message-ID|range pattern [pattern ...]" },
-    {	NULL,           CMD_unimp,      false,  0,      0,
+    {	NULL,           CMD_unimp,      false,  0,      0,      true,
         NULL }
 };
 
@@ -1305,7 +1308,7 @@
 		continue;
 	    if (Tracing)
 		syslog(L_TRACE, "%s < %s", Client.host, PushedBack);
-	    ac = Argify(PushedBack, &av);
+	    ac = nArgify(PushedBack, &av, 1);
 	    r = RTok;
 	}
 	else {
@@ -1333,23 +1336,30 @@
 		    memcpy(buff, p, len + 1);
 		    /* Do some input processing, check for blank line. */
                     if (buff[0] != '\0')
-                        ac = Argify(buff, &av);
+                        ac = nArgify(buff, &av, 1);
 		    if (Tracing) {
                         /* Do not log passwords if AUTHINFO PASS,
                          * AUTHINFO SASL PLAIN or AUTHINFO SASL EXTERNAL
-                         * are used. 
+                         * are used.  (Only one space between SASL and
+                         * PLAIN/EXTERNAL should be put; otherwise, the
+                         * whole command will be logged).
                          * AUTHINFO SASL LOGIN does not use an initial response;
                          * therefore, there is nothing to hide here. */
-                        if (ac > 2 && strcasecmp(av[0], "AUTHINFO") == 0
-                            && (strcasecmp(av[1], "PASS") == 0
-                                || (ac > 3 && strcasecmp(av[1], "SASL") == 0
-                                    && (strcasecmp(av[2], "PLAIN") == 0
-                                        || strcasecmp(av[2], "EXTERNAL") == 0))))
-                            syslog(L_TRACE, "%s < %s %s %s ********", Client.host,
-                                   av[0], av[1],
-                                   strcasecmp(av[1], "SASL") == 0 ? av[2] : "");
-                        else
+                        if (ac > 1 && strcasecmp(av[0], "AUTHINFO") == 0) {
+                            if (strncasecmp(av[1], "PASS", 4) == 0)
+                                syslog(L_TRACE, "%s < AUTHINFO PASS ********",
+                                       Client.host);
+                            else if (strncasecmp(av[1], "SASL PLAIN", 10) == 0)
+                                syslog(L_TRACE, "%s < AUTHINFO SASL PLAIN ********",
+                                       Client.host);
+                            else if (strncasecmp(av[1], "SASL EXTERNAL", 13) == 0)
+                                syslog(L_TRACE, "%s < AUTHINFO SASL EXTERNAL ********",
+                                       Client.host);
+                            else
+                                syslog(L_TRACE, "%s < %s", Client.host, buff);
+                        } else {
                             syslog(L_TRACE, "%s < %s", Client.host, buff);
+                        }
                     }
 		    if (buff[0] == '\0')
 			continue;
@@ -1360,7 +1370,7 @@
                 /* The line is too long but we have to make sure that
                  * no recognized command has been sent. */
                 q = (char *)p;
-                ac = Argify(q, &av);
+                ac = nArgify(q, &av, 1);
                 validcommandtoolong = false;
                 for (cp = CMDtable; cp->Name; cp++) {
                     if ((cp->Function != CMD_unimp) &&
@@ -1398,6 +1408,12 @@
 	    continue;
 	}
 
+        /* Go on parsing the command. */
+        ac--;
+        ac += reArgify(av[ac], &av[ac],
+                       cp->Stripspaces ? -1 : cp->Minac - ac - 1,
+                       cp->Stripspaces);
+
         /* Check whether all arguments do not exceed their allowed size. */
         if (ac > 1) {
             validcommandtoolong = false;




More information about the inn-committers mailing list