Fix buffer overflow by user supplied data

Passing very long usernames/passwords for pam authentication could
possibly lead to a stack based buffer overrun in the auth-pam plugin.

Adds a dependency to C99 (includes stdbool.h)

Signed-off-by: Jens Neuhalfen <jens@neuhalfen.name>
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <A4F03DE4-3E70-4815-B4B4-CC185E35CF2C@neuhalfen.name>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11477
Signed-off-by: Gert Doering <gert@greenie.muc.de>

1 files changed, 26 insertions, 4 deletions

diff --git a/src/plugins/auth-pam/auth-pam.c b/src/plugins/auth-pam/auth-pam.c
index 95692ab..710accc 100644
--- a/src/plugins/auth-pam/auth-pam.c
+++ b/src/plugins/auth-pam/auth-pam.c
@@ -39,6 +39,7 @@
 #include <stdio.h>
 #include <string.h>
 #include <ctype.h>
+#include <stdbool.h>
 #include <unistd.h>
 #include <stdlib.h>
 #include <sys/types.h>
@@ -47,6 +48,7 @@
 #include <fcntl.h>
 #include <signal.h>
 #include <syslog.h>
+#include <stdint.h>
 
 #include <openvpn-plugin.h>
 
@@ -119,17 +121,37 @@ static void pam_server (int fd, const char *service, int verb, const struct name
  *  a pointer to the NEW string.  Does not modify the input strings.  Will not enter an
  *  infinite loop with clever 'searchfor' and 'replacewith' strings.
  *  Daniel Johnson - Progman2000@usa.net / djohnson@progman.us
+ *
+ *  Retuns NULL when
+ *   - any parameter is NULL
+ *   - the worst-case result is to large ( >= SIZE_MAX)
  */
 static char *
 searchandreplace(const char *tosearch, const char *searchfor, const char *replacewith)
 {
+  if (!tosearch || !searchfor || !replacewith) return NULL;
+
+  size_t tosearchlen = strlen(tosearch);
+  size_t replacewithlen = strlen(replacewith);
+  size_t templen = tosearchlen * replacewithlen;
+
+  if (tosearchlen == 0 || strlen(searchfor) == 0 || replacewithlen == 0) {
+    return NULL;
+  }
+
+  bool is_potential_integer_overflow =  (templen == SIZE_MAX) || (templen / tosearchlen != replacewithlen);
+
+  if (is_potential_integer_overflow) {
+       return NULL;
+  }
+
+  // state: all parameters are valid
+
   const char *searching=tosearch;
   char *scratch;
-  char temp[strlen(tosearch)*10];
-  temp[0]=0;
 
-  if (!tosearch || !searchfor || !replacewith) return 0;
-  if (!strlen(tosearch) || !strlen(searchfor) || !strlen(replacewith)) return 0;
+  char temp[templen+1];
+  temp[0]=0;
 
   scratch = strstr(searching,searchfor);
   if (!scratch) return strdup(tosearch);