search

GNS3 / iouyap

Bridge IOU to UDP, TAP and Ethernet.
GNU General Public License v3.0
24 stars 12 forks source link

buffer overflow via stack and heap overflows via sprintf(...,"%s",...) #8

Closed anubisg1 closed 9 years ago

anubisg1 commented 9 years ago

Security audit handled by the openSUSE security team ( https://bugzilla.suse.com/show_bug.cgi?id=904060) found "a lot of stack and heap overflows via sprintf(...,"%s",...) calls with input from config files (sprintf in .y file) and command line args."

to follow the patch that should fix the issue. Please review and merge upstream.

diff --git a/Makefile b/Makefile
index 21b6cce..bc8df5a 100644
--- a/Makefile
+++ b/Makefile
@@ -22,7 +22,7 @@ SHELL = /bin/sh

 srcdir = .

-CC = gcc #-O3
+CC = gcc -I . #-O3
 CDEBUG = -g -DDEBUG
 CFLAGS = $(CDEBUG) -Wall

diff --git a/config.c b/config.c
index 2ae6854..81351a5 100644
--- a/config.c
+++ b/config.c
@@ -27,12 +27,12 @@ extern short yap_appl_id;
 extern dictionary *yap_config;

-#define KEY_DEF(key, param) \
-  sprintf(key, "%s:%s", DEFAULT_SECTION, param)
-#define KEY_ID(key, param)  \
-  sprintf(key, "%d:%s", yap_appl_id, param)
-#define KEY_PORT(key, port, param) \
-  sprintf(key, "%d:%s:%s", yap_appl_id, port, param)
+#define KEY_DEF(key, n, param) \
+  snprintf(key, (n), "%s:%s", DEFAULT_SECTION, param)
+#define KEY_ID(key, n, param)  \
+  snprintf(key, (n), "%d:%s", yap_appl_id, param)
+#define KEY_PORT(key, n, port, param) \
+  snprintf(key, (n), "%d:%s:%s", yap_appl_id, port, param)

 int
@@ -47,7 +47,7 @@ ini_find_default (char *param)
 {
   char key[MAX_KEY_SIZE];

-  KEY_DEF (key, param);
+  KEY_DEF (key, sizeof(key), param);
   return ini_find (key);
 }

@@ -57,7 +57,7 @@ ini_find_id (char *param)
 {
   char key[MAX_KEY_SIZE];

-  KEY_ID (key, param);
+  KEY_ID (key, sizeof(key), param);
   return ini_find (key);
 }

@@ -67,7 +67,7 @@ ini_find_port (char *port, char *param)
 {
   char key[MAX_KEY_SIZE];

-  KEY_PORT (key, port, param);
+  KEY_PORT (key, sizeof(key), port, param);
   return ini_find (key);
 }

@@ -88,7 +88,7 @@ ini_getstr_default (char **value, char *param)
 {
   char key[MAX_KEY_SIZE];

-  KEY_DEF (key, param);
+  KEY_DEF (key, sizeof(key), param);
   if (ini_getstr (value, key))
     return 1;
   return 0;
@@ -100,7 +100,7 @@ ini_getstr_id (char **value, char *param)
 {
   char key[MAX_KEY_SIZE];

-  KEY_ID (key, param);
+  KEY_ID (key, sizeof(key), param);
   if (ini_getstr (value, key))
     return 1;
   return ini_getstr_default (value, param);
@@ -112,7 +112,7 @@ ini_getstr_port (char **value, char *port, char *param)
 {
   char key[MAX_KEY_SIZE];

-  KEY_PORT (key, port, param);
+  KEY_PORT (key, sizeof(key), port, param);
   if (ini_getstr (value, key))
     return 1;
   return ini_getstr_id (value, param);
@@ -168,7 +168,7 @@ ini_getint_default (int *value, char *param)
 {
   char key[MAX_KEY_SIZE];

-  KEY_DEF (key, param);
+  KEY_DEF (key, sizeof(key), param);
   if (ini_getint (value, key))
     return 1;
   return 0;
@@ -180,7 +180,7 @@ ini_getint_id (int *value, char *param)
 {
   char key[MAX_KEY_SIZE];

-  KEY_ID (key, param);
+  KEY_ID (key, sizeof(key), param);
   if (ini_getint (value, key))
     return 1;
   return ini_getint_default (value, param);
@@ -192,7 +192,7 @@ ini_getint_port (int *value, char *port, char *param)
 {
   char key[MAX_KEY_SIZE];

-  KEY_PORT (key, port, param);
+  KEY_PORT (key, sizeof(key), port, param);
   if (ini_getint (value, key))
     return 1;
   return ini_getint_id (value, param);
@@ -248,7 +248,7 @@ ini_getbool_default (int *value, char *param)
 {
   char key[MAX_KEY_SIZE];

-  KEY_DEF (key, param);
+  KEY_DEF (key, sizeof(key), param);
   if (ini_getbool (value, key))
     return 1;
   return 0;
@@ -260,7 +260,7 @@ ini_getbool_id (int *value, char *param)
 {
   char key[MAX_KEY_SIZE];

-  KEY_ID (key, param);
+  KEY_ID (key, sizeof(key), param);
   if (ini_getbool (value, key))
     return 1;
   return ini_getbool_default (value, param);
@@ -272,7 +272,7 @@ ini_getbool_port (int *value, char *port, char *param)
 {
   char key[MAX_KEY_SIZE];

-  KEY_PORT (key, port, param);
+  KEY_PORT (key, sizeof(key), port, param);
   if (ini_getbool (value, key))
     return 1;
   return ini_getbool_id (value, param);
@@ -328,7 +328,7 @@ ini_getdouble_default (double *value, char *param)
 {
   char key[MAX_KEY_SIZE];

-  KEY_DEF (key, param);
+  KEY_DEF (key, sizeof(key), param);
   if (ini_getdouble (value, key))
     return 1;
   return 0;
@@ -340,7 +340,7 @@ ini_getdouble_id (double *value, char *param)
 {
   char key[MAX_KEY_SIZE];

-  KEY_ID (key, param);
+  KEY_ID (key, sizeof(key), param);
   if (ini_getdouble (value, key))
     return 1;
   return ini_getdouble_default (value, param);
@@ -352,7 +352,7 @@ ini_getdouble_port (double *value, char *port, char *param)
 {
   char key[MAX_KEY_SIZE];

-  KEY_PORT (key, port, param);
+  KEY_PORT (key, sizeof(key), port, param);
   if (ini_getdouble (value, key))
     return 1;
   return ini_getdouble_id (value, param);
diff --git a/iouyap.c b/iouyap.c
index f487d46..c15893d 100644
--- a/iouyap.c
+++ b/iouyap.c
@@ -179,7 +179,7 @@ lock_socket (const char *name)

   // We have the lock. Wipe out the file and put our PID in it.
   ftruncate (fd, 0);
-  pid_len = sprintf (pid, "%ld\n", (long) getpid ());
+  pid_len = snprintf (pid, sizeof(pid), "%ld\n", (long) getpid ());
   if (write (fd, pid, pid_len) == -1)
     {
       e = errno;
@@ -1046,7 +1046,7 @@ open_iou_udp ()
   hints.ai_next = NULL;

   // TODO: allow binding to a specific IP address
-  sprintf (local_port, "%u", get_iou_udp_port (yap_appl_id));
+  snprintf (local_port, sizeof(local_port), "%u", get_iou_udp_port (yap_appl_id));
   if (getaddrinfo (NULL, local_port, &hints, &result) != 0)
     fatal_error ("getaddrinfo");

@@ -1215,8 +1215,8 @@ create_foreign_threads (pthread_attr_t * thread_attrs,
       port_table[i].pcap_fd = NO_FD;

       port = unpack_port (i);
-      sprintf (port_key, "%d/%d", port.bay, port.unit);
-      sprintf (key, "%d:%s", yap_appl_id, port_key);
+      snprintf (port_key, sizeof(port_key), "%d/%d", port.bay, port.unit);
+      snprintf (key, sizeof(key), "%d:%s", yap_appl_id, port_key);

       /* Don't bother if the section doesn't even exist */
       if (!ini_find (key))
@@ -1545,7 +1545,7 @@ main (int argc, char **argv)
           iniparser_set (yap_config, cmdline_node, NULL);

           /* Now create the key=value pair */
-          sprintf (key, "%s:%s", cmdline_node, cmdline_dev_type);
+          snprintf (key, sizeof(key), "%s:%s", cmdline_node, cmdline_dev_type);
           iniparser_set (yap_config, key, cmdline_dev);

           free (cmdline_node);
diff --git a/netmap_parse.y b/netmap_parse.y
index c8478cd..b6865a4 100644
--- a/netmap_parse.y
+++ b/netmap_parse.y
@@ -130,7 +130,7 @@ host
     | '@' ADDRESS               {
                                     struct hostent *host;

-                                    sprintf($$, "@%s", $2);
+                                    snprintf($$, sizeof(yyval.pval), "@%s", $2);
                                     host = gethostbyname ($2);
                                     if (host == NULL)
                                       {