Pass a buffer size to InterpretArg() and do not let it overflow it, saves

propolice some work in command_Expand_Interpret().

Inspired by FreeBSD; ok deraadt@

3 files changed, 37 insertions, 14 deletions

diff --git a/usr.sbin/ppp/ppp/command.c b/usr.sbin/ppp/ppp/command.c
index 7d88f3e56b9..5c45b9050c3 100644
--- a/usr.sbin/ppp/ppp/command.c
+++ b/usr.sbin/ppp/ppp/command.c
@@ -25,7 +25,7 @@
  * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
  *
- * $OpenBSD: command.c,v 1.91 2005/09/21 16:28:47 brad Exp $
+ * $OpenBSD: command.c,v 1.92 2008/03/02 18:46:32 miod Exp $
  */
 
 #include <sys/param.h>
@@ -1130,7 +1130,10 @@ command_Expand_Interpret(char *buff, int nb, char *argv[MAXARGS], int offset)
 {
   char buff2[LINE_LEN-offset];
 
-  InterpretArg(buff, buff2);
+  if (InterpretArg(buff, buff2, sizeof buff2) == NULL) {
+    log_Printf(LogWARN, "Failed to expand command '%s': too long for the destination buffer\n", buff);
+    return -1;
+  }
   strncpy(buff, buff2, LINE_LEN - offset - 1);
   buff[LINE_LEN - offset - 1] = '\0';
 
diff --git a/usr.sbin/ppp/ppp/systems.c b/usr.sbin/ppp/ppp/systems.c
index f940967e473..cdd39a1c001 100644
--- a/usr.sbin/ppp/ppp/systems.c
+++ b/usr.sbin/ppp/ppp/systems.c
@@ -25,7 +25,7 @@
  * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
  *
- * $OpenBSD: systems.c,v 1.18 2002/05/16 01:13:39 brian Exp $
+ * $OpenBSD: systems.c,v 1.19 2008/03/02 18:46:33 miod Exp $
  */
 
 #include <sys/param.h>
@@ -64,9 +64,12 @@ CloseSecret(FILE *fp)
   fclose(fp);
 }
 
-/* Move string from ``from'' to ``to'', interpreting ``~'' and $.... */
+/*
+ * Move string from ``from'' to ``to'', interpreting ``~'' and $....
+ * Returns NULL if string expansion failed due to lack of buffer space.
+ */
 const char *
-InterpretArg(const char *from, char *to)
+InterpretArg(const char *from, char *to, size_t tosiz)
 {
   char *ptr, *startto, *endto;
   struct passwd *pwd;
@@ -75,12 +78,14 @@ InterpretArg(const char *from, char *to)
 
   instring = 0;
   startto = to;
-  endto = to + LINE_LEN - 1;
+  endto = to + tosiz - 1;
 
   while(issep(*from))
     from++;
 
   while (*from != '\0') {
+    if (to >= endto)
+      return NULL;
     switch (*from) {
       case '"':
         instring = !instring;
@@ -96,6 +101,8 @@ InterpretArg(const char *from, char *to)
             *to++ = '\\';	/* Pass the escapes on, maybe skipping \# */
             break;
         }
+        if (to >= endto)
+          return NULL;
         *to++ = *from++;
         break;
       case '$':
@@ -106,8 +113,8 @@ InterpretArg(const char *from, char *to)
           ptr = strchr(from+2, '}');
           if (ptr) {
             len = ptr - from - 2;
-            if (endto - to < len )
-              len = endto - to;
+            if (endto - to < len)
+              return NULL;
             if (len) {
               strncpy(to, from+2, len);
               to[len] = '\0';
@@ -126,9 +133,13 @@ InterpretArg(const char *from, char *to)
             *ptr++ = *from;
           *ptr = '\0';
         }
+        if (to >= endto)
+          return NULL;
         if (*to == '\0')
           *to++ = '$';
         else if ((env = getenv(to)) != NULL) {
+          if (endto - to < strlen(env))
+            return NULL;
           strncpy(to, env, endto - to);
           *endto = '\0';
           to += strlen(to);
@@ -141,19 +152,24 @@ InterpretArg(const char *from, char *to)
         if (len == 0)
           pwd = getpwuid(ID0realuid());
         else {
+          if (endto - to < len)
+            return NULL;
           strncpy(to, from, len);
           to[len] = '\0';
           pwd = getpwnam(to);
         }
+        if (to >= endto)
+          return NULL;
         if (pwd == NULL)
           *to++ = '~';
         else {
+          if (endto - to < strlen(pwd->pw_dir))
+            return NULL;
           strncpy(to, pwd->pw_dir, endto - to);
           *endto = '\0';
           to += strlen(to);
           from += len;
         }
-        endpwent();
         break;
 
       default:
@@ -178,12 +194,16 @@ InterpretArg(const char *from, char *to)
 #define CTRL_INCLUDE (1)
 
 static int
-DecodeCtrlCommand(char *line, char *arg)
+DecodeCtrlCommand(char *line, char *arg, size_t argsiz)
 {
   const char *end;
 
   if (!strncasecmp(line, "include", 7) && issep(line[7])) {
-    end = InterpretArg(line+8, arg);
+    end = InterpretArg(line+8, arg, argsiz);
+    if (end == NULL) {
+      log_Printf(LogWARN, "Failed to expand command '%s': too long for the destination buffer\n", line);
+      return CTRL_UNKNOWN;
+    }
     if (*end && *end != '#')
       log_Printf(LogWARN, "usage: !include filename\n");
     else
@@ -352,7 +372,7 @@ ReadSystem(struct bundle *bundle, const char *name, const char *file,
       break;
 
     case '!':
-      switch (DecodeCtrlCommand(cp+1, arg)) {
+      switch (DecodeCtrlCommand(cp+1, arg, LINE_LEN)) {
       case CTRL_INCLUDE:
         log_Printf(LogCOMMAND, "%s: Including \"%s\"\n", filename, arg);
         n = ReadSystem(bundle, name, arg, prompt, cx, how);
diff --git a/usr.sbin/ppp/ppp/systems.h b/usr.sbin/ppp/ppp/systems.h
index a5cce1aa954..af358340947 100644
--- a/usr.sbin/ppp/ppp/systems.h
+++ b/usr.sbin/ppp/ppp/systems.h
@@ -25,7 +25,7 @@
  * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
  *
- * $OpenBSD: systems.h,v 1.7 2001/11/23 11:17:03 brian Exp $
+ * $OpenBSD: systems.h,v 1.8 2008/03/02 18:46:33 miod Exp $
  */
 
 struct prompt;
@@ -40,4 +40,4 @@ extern FILE *OpenSecret(const char *);
 extern void CloseSecret(FILE *);
 extern int AllowUsers(struct cmdargs const *);
 extern int AllowModes(struct cmdargs const *);
-extern const char *InterpretArg(const char *, char *);
+extern const char *InterpretArg(const char *, char *, size_t);