Notice and warn about unterminated quoted strings in commands.

The entire command is ignored if the syntax is invalid...

13 files changed, 100 insertions, 44 deletions

diff --git a/usr.sbin/ppp/README.changes b/usr.sbin/ppp/README.changes
index 11d83d6..99252d8 100644
--- a/usr.sbin/ppp/README.changes
+++ b/usr.sbin/ppp/README.changes
@@ -96,3 +96,5 @@ o The ``set autoload'' command syntax and implementation have changed as the
 o Ppp now waits either the full ``set cd'' time or until carrier is detected
   before running the login script (whichever comes first).
 o The -alias flag has been deprecated.  The -nat flag should be used instead.
+o Unbalanced quotes in commands are now warned about and the entire command
+  is ignored.
diff --git a/usr.sbin/ppp/auth.c b/usr.sbin/ppp/auth.c
index e6020d4..7c66fcd 100644
--- a/usr.sbin/ppp/auth.c
+++ b/usr.sbin/ppp/auth.c
@@ -104,18 +104,21 @@ int
 auth_SetPhoneList(const char *name, char *phone, int phonelen)
 {
   FILE *fp;
-  int n;
+  int n, lineno;
   char *vector[6];
   char buff[LINE_LEN];
 
   fp = OpenSecret(SECRETFILE);
+  lineno = 0;
   if (fp != NULL) {
     while (fgets(buff, sizeof buff, fp)) {
+      lineno++;
       if (buff[0] == '#')
         continue;
       buff[strlen(buff) - 1] = '\0';
       memset(vector, '\0', sizeof vector);
-      n = MakeArgs(buff, vector, VECSIZE(vector));
+      if ((n = MakeArgs(buff, vector, VECSIZE(vector))) < 0)
+        log_Printf(LogWARN, "%s: %d: Invalid line\n", SECRETFILE, lineno);
       if (n < 5)
         continue;
       if (strcmp(vector[0], name) == 0) {
@@ -137,7 +140,7 @@ int
 auth_Select(struct bundle *bundle, const char *name)
 {
   FILE *fp;
-  int n;
+  int n, lineno;
   char *vector[5];
   char buff[LINE_LEN];
 
@@ -157,13 +160,16 @@ auth_Select(struct bundle *bundle, const char *name)
 #endif
 
   fp = OpenSecret(SECRETFILE);
+  lineno = 0;
   if (fp != NULL) {
     while (fgets(buff, sizeof buff, fp)) {
+      lineno++;
       if (buff[0] == '#')
         continue;
       buff[strlen(buff) - 1] = '\0';
       memset(vector, '\0', sizeof vector);
-      n = MakeArgs(buff, vector, VECSIZE(vector));
+      if ((n = MakeArgs(buff, vector, VECSIZE(vector))) < 0)
+        log_Printf(LogWARN, "%s: %d: Invalid line\n", SECRETFILE, lineno);
       if (n < 2)
         continue;
       if (strcmp(vector[0], name) == 0) {
@@ -208,18 +214,21 @@ auth_Validate(struct bundle *bundle, const char *name,
   /* Used by PAP routines */
 
   FILE *fp;
-  int n;
+  int n, lineno;
   char *vector[5];
   char buff[LINE_LEN];
 
   fp = OpenSecret(SECRETFILE);
+  lineno = 0;
   if (fp != NULL) {
     while (fgets(buff, sizeof buff, fp)) {
+      lineno++;
       if (buff[0] == '#')
         continue;
       buff[strlen(buff) - 1] = 0;
       memset(vector, '\0', sizeof vector);
-      n = MakeArgs(buff, vector, VECSIZE(vector));
+      if ((n = MakeArgs(buff, vector, VECSIZE(vector))) < 0)
+        log_Printf(LogWARN, "%s: %d: Invalid line\n", SECRETFILE, lineno);
       if (n < 2)
         continue;
       if (strcmp(vector[0], name) == 0) {
@@ -245,7 +254,7 @@ auth_GetSecret(struct bundle *bundle, const char *name, int len,
   /* Used by CHAP routines */
 
   FILE *fp;
-  int n;
+  int n, lineno;
   char *vector[5];
   static char buff[LINE_LEN];	/* vector[] will point here when returned */
 
@@ -253,14 +262,17 @@ auth_GetSecret(struct bundle *bundle, const char *name, int len,
   if (fp == NULL)
     return (NULL);
 
+  lineno = 0;
   while (fgets(buff, sizeof buff, fp)) {
+    lineno++;
     if (buff[0] == '#')
       continue;
     n = strlen(buff) - 1;
     if (buff[n] == '\n')
       buff[n] = '\0';	/* Trim the '\n' */
     memset(vector, '\0', sizeof vector);
-    n = MakeArgs(buff, vector, VECSIZE(vector));
+    if ((n = MakeArgs(buff, vector, VECSIZE(vector))) < 0)
+      log_Printf(LogWARN, "%s: %d: Invalid line\n", SECRETFILE, lineno);
     if (n < 2)
       continue;
     if (strlen(vector[0]) == len && strncmp(vector[0], name, len) == 0) {
diff --git a/usr.sbin/ppp/chap.c b/usr.sbin/ppp/chap.c
index e28c509..8101413 100644
--- a/usr.sbin/ppp/chap.c
+++ b/usr.sbin/ppp/chap.c
@@ -230,6 +230,15 @@ chap_StartChild(struct chap *chap, char *prog, const char *name)
 
     case 0:
       timer_TermService();
+
+      if ((argc = command_Interpret(prog, strlen(prog), argv)) <= 0) {
+        if (argc < 0) {
+          log_Printf(LogWARN, "CHAP: Invalid command syntax\n");
+          _exit(255);
+        }
+        _exit(0);
+      }
+
       close(in[1]);
       close(out[0]);
       if (out[1] == STDIN_FILENO)
@@ -245,7 +254,6 @@ chap_StartChild(struct chap *chap, char *prog, const char *name)
       for (fd = getdtablesize(); fd > STDERR_FILENO; fd--)
         fcntl(fd, F_SETFD, 1);
       setuid(geteuid());
-      argc = command_Interpret(prog, strlen(prog), argv);
       command_Expand(nargv, argc, (char const *const *)argv,
                      chap->auth.physical->dl->bundle, 0, pid);
       execvp(nargv[0], nargv);
diff --git a/usr.sbin/ppp/chat.c b/usr.sbin/ppp/chat.c
index 0ccda30..ce7899e 100644
--- a/usr.sbin/ppp/chat.c
+++ b/usr.sbin/ppp/chat.c
@@ -550,7 +550,7 @@ chat_Init(struct chat *c, struct physical *p)
   memset(&c->timeout, '\0', sizeof c->timeout);
 }
 
-void
+int
 chat_Setup(struct chat *c, const char *data, const char *phone)
 {
   c->state = CHAT_EXPECT;
@@ -561,7 +561,7 @@ chat_Setup(struct chat *c, const char *data, const char *phone)
   } else {
     strncpy(c->script, data, sizeof c->script - 1);
     c->script[sizeof c->script - 1] = '\0';
-    c->argc =  MakeArgs(c->script, c->argv, VECSIZE(c->argv));
+    c->argc = MakeArgs(c->script, c->argv, VECSIZE(c->argv));
   }
 
   c->arg = -1;
@@ -575,6 +575,8 @@ chat_Setup(struct chat *c, const char *data, const char *phone)
 
   timer_Stop(&c->pause);
   timer_Stop(&c->timeout);
+
+  return c->argc >= 0;
 }
 
 void
@@ -700,7 +702,12 @@ ExecStr(struct physical *physical, char *command, char *out, int olen)
   int stat, nb, argc, i;
 
   log_Printf(LogCHAT, "Exec: %s\n", command);
-  argc = MakeArgs(command, vector, VECSIZE(vector));
+  if ((argc = MakeArgs(command, vector, VECSIZE(vector))) <= 0) {
+    if (argc < 0)
+      log_Printf(LogWARN, "Syntax error in exec command\n");
+    *out = '\0';
+    return;
+  }
   command_Expand(argv, argc, (char const *const *)vector,
                  physical->dl->bundle, 0, getpid());
 
diff --git a/usr.sbin/ppp/chat.h b/usr.sbin/ppp/chat.h
index 872e1c6..2e40bab 100644
--- a/usr.sbin/ppp/chat.h
+++ b/usr.sbin/ppp/chat.h
@@ -77,6 +77,6 @@ struct chat {
 #define	VECSIZE(v)	(sizeof(v) / sizeof(v[0]))
 
 extern void chat_Init(struct chat *, struct physical *);
-extern void chat_Setup(struct chat *, const char *, const char *);
+extern int chat_Setup(struct chat *, const char *, const char *);
 extern void chat_Finish(struct chat *);
 extern void chat_Destroy(struct chat *);
diff --git a/usr.sbin/ppp/command.c b/usr.sbin/ppp/command.c
index cae68b2..d2add9d 100644
--- a/usr.sbin/ppp/command.c
+++ b/usr.sbin/ppp/command.c
@@ -976,15 +976,18 @@ command_Run(struct bundle *bundle, int argc, char const *const *argv,
   }
 }
 
-void
+int
 command_Decode(struct bundle *bundle, char *buff, int nb, struct prompt *prompt,
               const char *label)
 {
   int argc;
   char *argv[MAXARGS];
 
-  argc = command_Interpret(buff, nb, argv);
+  if ((argc = command_Interpret(buff, nb, argv)) < 0)
+    return 0;
+
   command_Run(bundle, argc, (char const *const *)argv, prompt, label, NULL);
+  return 1;
 }
 
 static int
diff --git a/usr.sbin/ppp/command.h b/usr.sbin/ppp/command.h
index 6b1468d..d08f67d 100644
--- a/usr.sbin/ppp/command.h
+++ b/usr.sbin/ppp/command.h
@@ -58,7 +58,7 @@ extern void command_Expand(char **, int, char const *const *, struct bundle *,
 extern int command_Interpret(char *, int, char *vector[MAXARGS]);
 extern void command_Run(struct bundle *, int, char const *const *,
                         struct prompt *, const char *, struct datalink *);
-extern void command_Decode(struct bundle *, char *, int, struct prompt *,
+extern int command_Decode(struct bundle *, char *, int, struct prompt *,
                            const char *);
 extern struct link *command_ChooseLink(struct cmdargs const *);
 extern const char *command_ShowNegval(unsigned);
diff --git a/usr.sbin/ppp/datalink.c b/usr.sbin/ppp/datalink.c
index b5e20f9..673de85 100644
--- a/usr.sbin/ppp/datalink.c
+++ b/usr.sbin/ppp/datalink.c
@@ -214,7 +214,8 @@ datalink_LoginDone(struct datalink *dl)
     log_Printf(LogWARN, "datalink_LoginDone: Not connected.\n");
     if (dl->script.run) { 
       datalink_NewState(dl, DATALINK_LOGOUT);
-      chat_Setup(&dl->chat, dl->cfg.script.logout, NULL);
+      if (!chat_Setup(&dl->chat, dl->cfg.script.logout, NULL))
+        log_Printf(LogWARN, "Invalid logout script\n");
     } else {
       physical_StopDeviceTimer(dl->physical);
       if (dl->physical->type == PHYS_DEDICATED)
@@ -273,8 +274,10 @@ datalink_UpdateSet(struct descriptor *d, fd_set *r, fd_set *w, fd_set *e,
                            dl->physical->name.full);
           if (dl->script.run) {
             datalink_NewState(dl, DATALINK_DIAL);
-            chat_Setup(&dl->chat, dl->cfg.script.dial, *dl->cfg.script.dial ?
-                       datalink_ChoosePhoneNumber(dl) : "");
+            if (!chat_Setup(&dl->chat, dl->cfg.script.dial,
+                            *dl->cfg.script.dial ?
+                            datalink_ChoosePhoneNumber(dl) : ""))
+              log_Printf(LogWARN, "Invalid dial script\n");
             if (!(dl->physical->type & (PHYS_DDIAL|PHYS_DEDICATED)) &&
                 dl->cfg.dial.max)
               log_Printf(LogCHAT, "%s: Dial attempt %u of %d\n",
@@ -322,7 +325,8 @@ datalink_UpdateSet(struct descriptor *d, fd_set *r, fd_set *w, fd_set *e,
         case CARRIER_OK:
           if (dl->script.run) {
             datalink_NewState(dl, DATALINK_LOGIN);
-            chat_Setup(&dl->chat, dl->cfg.script.login, NULL);
+            if (!chat_Setup(&dl->chat, dl->cfg.script.login, NULL))
+              log_Printf(LogWARN, "Invalid login script\n");
           } else
             datalink_LoginDone(dl);
           return datalink_UpdateSet(d, r, w, e, n);
@@ -331,7 +335,8 @@ datalink_UpdateSet(struct descriptor *d, fd_set *r, fd_set *w, fd_set *e,
           physical_Offline(dl->physical);	/* Is this required ? */
           if (dl->script.run) { 
             datalink_NewState(dl, DATALINK_HANGUP);
-            chat_Setup(&dl->chat, dl->cfg.script.hangup, NULL);
+            if (!chat_Setup(&dl->chat, dl->cfg.script.hangup, NULL))
+              log_Printf(LogWARN, "Invalid hangup script\n");
             return datalink_UpdateSet(d, r, w, e, n);
           } else {
             datalink_HangupDone(dl);
@@ -357,7 +362,8 @@ datalink_UpdateSet(struct descriptor *d, fd_set *r, fd_set *w, fd_set *e,
             case DATALINK_LOGOUT:
               datalink_NewState(dl, DATALINK_HANGUP);
               physical_Offline(dl->physical);
-              chat_Setup(&dl->chat, dl->cfg.script.hangup, NULL);
+              if (!chat_Setup(&dl->chat, dl->cfg.script.hangup, NULL))
+                log_Printf(LogWARN, "Invalid hangup script\n");
               return datalink_UpdateSet(d, r, w, e, n);
             case DATALINK_LOGIN:
               dl->phone.alt = NULL;
@@ -377,7 +383,8 @@ datalink_UpdateSet(struct descriptor *d, fd_set *r, fd_set *w, fd_set *e,
             case DATALINK_LOGIN:
               datalink_NewState(dl, DATALINK_HANGUP);
               physical_Offline(dl->physical);
-              chat_Setup(&dl->chat, dl->cfg.script.hangup, NULL);
+              if (!chat_Setup(&dl->chat, dl->cfg.script.hangup, NULL))
+                log_Printf(LogWARN, "Invalid hangup script\n");
               return datalink_UpdateSet(d, r, w, e, n);
           }
           break;
@@ -511,10 +518,12 @@ datalink_ComeDown(struct datalink *dl, int how)
     if (dl->script.run && dl->state != DATALINK_OPENING) {
       if (dl->state == DATALINK_LOGOUT) {
         datalink_NewState(dl, DATALINK_HANGUP);
-        chat_Setup(&dl->chat, dl->cfg.script.hangup, NULL);
+        if (!chat_Setup(&dl->chat, dl->cfg.script.hangup, NULL))
+          log_Printf(LogWARN, "Invalid hangup script\n");
       } else {
         datalink_NewState(dl, DATALINK_LOGOUT);
-        chat_Setup(&dl->chat, dl->cfg.script.logout, NULL);
+        if (!chat_Setup(&dl->chat, dl->cfg.script.logout, NULL))
+          log_Printf(LogWARN, "Invalid logout script\n");
       }
     } else
       datalink_HangupDone(dl);
diff --git a/usr.sbin/ppp/defs.c b/usr.sbin/ppp/defs.c
index 4cb6de6..6e7d5c9 100644
--- a/usr.sbin/ppp/defs.c
+++ b/usr.sbin/ppp/defs.c
@@ -276,6 +276,7 @@ findblank(char *p, int instring)
 	return (p);
       p++;
     }
+    return NULL;
   } else {
     while (*p) {
       if (issep(*p))
@@ -311,7 +312,9 @@ MakeArgs(char *script, char **pvect, int maxargs)
       *pvect++ = script;
       nargs++;
       script = findblank(script, instring);
-      if (*script)
+      if (script == NULL)
+        return -1;
+      else if (*script)
 	*script++ = '\0';
     }
   }
diff --git a/usr.sbin/ppp/exec.c b/usr.sbin/ppp/exec.c
index d1f3f64..0ded3cb 100644
--- a/usr.sbin/ppp/exec.c
+++ b/usr.sbin/ppp/exec.c
@@ -141,15 +141,20 @@ exec_Create(struct physical *p)
 
           log_Printf(LogDEBUG, "Exec'ing ``%s''\n", p->name.base);
 
+          if ((argc = MakeArgs(p->name.base, argv, VECSIZE(argv))) < 0) {
+            log_Printf(LogWARN, "Syntax error in exec command\n");
+            _exit(127);
+          }
+
+          command_Expand(argv, argc, (char const *const *)argv,
+                         p->dl->bundle, 0, realpid);
+
           dup2(fids[1], STDIN_FILENO);
           dup2(fids[1], STDOUT_FILENO);
           dup2(fids[1], STDERR_FILENO);
           for (i = getdtablesize(); i > STDERR_FILENO; i--)
             fcntl(i, F_SETFD, 1);
 
-          argc = MakeArgs(p->name.base, argv, VECSIZE(argv));
-          command_Expand(argv, argc, (char const *const *)argv,
-                         p->dl->bundle, 0, realpid);
           execvp(*argv, argv);
           printf("execvp failed: %s: %s\r\n", *argv, strerror(errno));
           _exit(127);
diff --git a/usr.sbin/ppp/prompt.c b/usr.sbin/ppp/prompt.c
index 5e39f00..5652453 100644
--- a/usr.sbin/ppp/prompt.c
+++ b/usr.sbin/ppp/prompt.c
@@ -190,7 +190,8 @@ prompt_Read(struct descriptor *d, struct bundle *bundle, const fd_set *fdset)
       if (n) {
         if ((op = log_PromptContext) == NULL)
           log_PromptContext = p;
-        command_Decode(bundle, linebuff, n, p, p->src.from);
+        if (!command_Decode(bundle, linebuff, n, p, p->src.from))
+          prompt_Printf(p, "Syntax error\n");
         log_PromptContext = op;
       }
     } else if (n <= 0) {
diff --git a/usr.sbin/ppp/radius.c b/usr.sbin/ppp/radius.c
index 366baec..e46baee 100644
--- a/usr.sbin/ppp/radius.c
+++ b/usr.sbin/ppp/radius.c
@@ -176,7 +176,10 @@ radius_Process(struct radius *r, int got)
         dest.ipaddr.s_addr = dest.mask.s_addr = INADDR_ANY;
         dest.width = 0;
         argc = command_Interpret(nuke, strlen(nuke), argv);
-        if (argc < 2)
+        if (argc < 0)
+          log_Printf(LogWARN, "radius: %s: Syntax error\n",
+                     argc == 1 ? argv[0] : "\"\"");
+        else if (argc < 2)
           log_Printf(LogWARN, "radius: %s: Invalid route\n",
                      argc == 1 ? argv[0] : "\"\"");
         else if ((strcasecmp(argv[0], "default") != 0 &&
diff --git a/usr.sbin/ppp/systems.c b/usr.sbin/ppp/systems.c
index db29092..5c8216d 100644
--- a/usr.sbin/ppp/systems.c
+++ b/usr.sbin/ppp/systems.c
@@ -340,19 +340,22 @@ ReadSystem(struct bundle *bundle, const char *name, const char *file,
           }
 
           len = strlen(cp);
-          argc = command_Interpret(cp, len, argv);
-          allowcmd = argc > 0 && !strcasecmp(argv[0], "allow");
-          if ((!(how == SYSTEM_EXEC) && allowcmd) ||
-              ((how == SYSTEM_EXEC) && !allowcmd)) {
-            /*
-             * Disable any context so that warnings are given to everyone,
-             * including syslog.
-             */
-            op = log_PromptContext;
-            log_PromptContext = NULL;
-	    command_Run(bundle, argc, (char const *const *)argv, prompt,
-                        name, cx);
-            log_PromptContext = op;
+          if ((argc = command_Interpret(cp, len, argv)) < 0)
+            log_Printf(LogWARN, "%s: %d: Syntax error\n", filename, linenum);
+          else {
+            allowcmd = argc > 0 && !strcasecmp(argv[0], "allow");
+            if ((!(how == SYSTEM_EXEC) && allowcmd) ||
+                ((how == SYSTEM_EXEC) && !allowcmd)) {
+              /*
+               * Disable any context so that warnings are given to everyone,
+               * including syslog.
+               */
+              op = log_PromptContext;
+              log_PromptContext = NULL;
+	      command_Run(bundle, argc, (char const *const *)argv, prompt,
+                          name, cx);
+              log_PromptContext = op;
+            }
           }
         }