Browse code

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>

Jens Neuhalfen authored on 2016/04/20 03:42:55
Showing 1 changed files
... ...
@@ -39,6 +39,7 @@
39 39
 #include <stdio.h>
40 40
 #include <string.h>
41 41
 #include <ctype.h>
42
+#include <stdbool.h>
42 43
 #include <unistd.h>
43 44
 #include <stdlib.h>
44 45
 #include <sys/types.h>
... ...
@@ -47,6 +48,7 @@
47 47
 #include <fcntl.h>
48 48
 #include <signal.h>
49 49
 #include <syslog.h>
50
+#include <stdint.h>
50 51
 
51 52
 #include <openvpn-plugin.h>
52 53
 
... ...
@@ -119,17 +121,37 @@ static void pam_server (int fd, const char *service, int verb, const struct name
119 119
  *  a pointer to the NEW string.  Does not modify the input strings.  Will not enter an
120 120
  *  infinite loop with clever 'searchfor' and 'replacewith' strings.
121 121
  *  Daniel Johnson - Progman2000@usa.net / djohnson@progman.us
122
+ *
123
+ *  Retuns NULL when
124
+ *   - any parameter is NULL
125
+ *   - the worst-case result is to large ( >= SIZE_MAX)
122 126
  */
123 127
 static char *
124 128
 searchandreplace(const char *tosearch, const char *searchfor, const char *replacewith)
125 129
 {
130
+  if (!tosearch || !searchfor || !replacewith) return NULL;
131
+
132
+  size_t tosearchlen = strlen(tosearch);
133
+  size_t replacewithlen = strlen(replacewith);
134
+  size_t templen = tosearchlen * replacewithlen;
135
+
136
+  if (tosearchlen == 0 || strlen(searchfor) == 0 || replacewithlen == 0) {
137
+    return NULL;
138
+  }
139
+
140
+  bool is_potential_integer_overflow =  (templen == SIZE_MAX) || (templen / tosearchlen != replacewithlen);
141
+
142
+  if (is_potential_integer_overflow) {
143
+       return NULL;
144
+  }
145
+
146
+  // state: all parameters are valid
147
+
126 148
   const char *searching=tosearch;
127 149
   char *scratch;
128
-  char temp[strlen(tosearch)*10];
129
-  temp[0]=0;
130 150
 
131
-  if (!tosearch || !searchfor || !replacewith) return 0;
132
-  if (!strlen(tosearch) || !strlen(searchfor) || !strlen(replacewith)) return 0;
151
+  char temp[templen+1];
152
+  temp[0]=0;
133 153
 
134 154
   scratch = strstr(searching,searchfor);
135 155
   if (!scratch) return strdup(tosearch);