Avoid buffer overruns and check return value of malloc()

diff --git a/lib/critical.c b/lib/critical.c
index 4cbb12d5dff2fb60a6437e993f31ce49044e3500..6b96880f8ce2a5b1d98c88582d20e390cc9d5f89 100644 (file)
--- a/lib/critical.c
+++ b/lib/critical.c
@@ -41,16 +41,10 @@ extern char *whoami;
 
 void critical_alert(char *instance, char *msg, ...)
 {
-  FILE *crit;                  /* FILE for critical log file */
-  char buf[BUFSIZ];            /* Holds the formatted message */
+  FILE *crit;
+  char *buf;
   va_list ap;
-
-  va_start(ap, msg);
-  vsprintf(buf, msg, ap);
-  va_end(ap);
-
-  /* Send zephyr notice */
-  send_zgram(instance, buf);
+  long start;
 
   /* Log message to critical file */
   if ((crit = fopen(CRITERRLOG, "a")))
@@ -62,21 +56,47 @@ void critical_alert(char *instance, char *msg, ...)
       time_s = ctime(&t) + 4;
       time_s[strlen(time_s) - 6] = '\0';
 
-      fprintf(crit, "%s <%ld> %s\n", time_s, (long)getpid(), buf);
+      fprintf(crit, "%s <%ld>", time_s, (long)getpid());
+      start = ftell(crit);
+      va_start(ap, msg);
+      vfprintf(crit, msg, ap);
+      va_end(ap);
+      fprintf(crit, "\n");
+
+      buf = malloc(ftell(crit) - start);
       fclose(crit);
+
+      if (buf)
+       {
+         va_start(ap, msg);
+         vsprintf(buf, msg, ap);
+         va_end(ap);
+
+         send_zgram(instance, buf);
+         com_err(whoami, 0, buf);
+
+         free(buf);
+       }
     }
 
-  com_err(whoami, 0, buf);
+  if (!crit || !buf)
+    {
+      send_zgram(instance, "Couldn't format critical syslog!");
+      com_err(whoami, 0, "Couldn't format critical syslog!");
+    }
 }
 
 
-
 /* Sends a zephyrgram of class "MOIRA", instance as a parameter.  Ignores
  * errors while sending message.
  */
 
 void send_zgram(char *inst, char *msg)
 {
+#ifdef SYSLOG
+  char *buf;
+#endif
+
 #ifdef ZEPHYR
   ZNotice_t znotice;
 
@@ -93,10 +113,12 @@ void send_zgram(char *inst, char *msg)
   ZSendNotice(&znotice, ZNOAUTH);
 #endif
 #ifdef SYSLOG
-  {
-    char buf[512];
-    sprintf(buf, "MOIRA: %s %s", inst, msg);
-    syslog(LOG_ERR, buf);
-  }
+  buf = malloc(9 + strlen(instance) + strlen(msg));
+  if (buf)
+    {
+      sprintf(buf, "MOIRA: %s %s", inst, msg);
+      syslog(LOG_ERR, buf);
+      free(buf);
+    }
 #endif
 }

diff --git a/lib/fixhost.c b/lib/fixhost.c
index c0c39ddf32324f269bd523179b959a7847b3a958..dc7789a956ebd9aabf01531e14928c25f9af212d 100644 (file)
--- a/lib/fixhost.c
+++ b/lib/fixhost.c
@@ -38,31 +38,32 @@ RCSID("$Header$");
 char *canonicalize_hostname(char *host)
 {
   struct hostent *hp;
-  int n_len;
+  int len;
   int has_dot = 0;
-  char tbuf[BUFSIZ];
-  struct utsname name;
-  char *cp;
+  char *tbuf, *cp;
 
-  if (strlen(host) > 2 && host[0] == '"' && host[strlen(host) - 1] == '"')
+  len = strlen(host);
+  if (len > 2 && host[0] == '"' && host[len - 1] == '"')
     {
-      strcpy(tbuf, host + 1);
+      tbuf = malloc(len - 1);
+      if (!tbuf)
+       return NULL;
+      strncpy(tbuf, host + 1, len - 2);
+      tbuf[len - 1] = '\0';
       free(host);
-      tbuf[strlen(tbuf) - 1] = '\0';
-      return strdup(tbuf);
+      return tbuf;
     }
 
-  if (strchr(host, '*') || strchr(host, '?') || strchr(host, '['))
+  if (strchr(host, '*') || strchr(host, '?'))
     return host;
 
   hp = gethostbyname(host);
 
   if (hp)
     {
-      n_len = strlen(hp->h_name) + 1;
-      host = realloc(host, n_len);
-
-      strcpy(host, hp->h_name);
+      host = realloc(host, strlen(hp->h_name) + 1);
+      if (host)
+       strcpy(host, hp->h_name);
       return host;
     }
   else
@@ -70,10 +71,9 @@ char *canonicalize_hostname(char *host)
       /* can't get name from nameserver; fix up the format a bit */
       for (cp = host; *cp; cp++)
        {
-         int c;
-         if (islower(c = *cp))
-           *cp = toupper(c);
-         has_dot |= (c == '.');
+         if (islower(*cp))
+           *cp = toupper(*cp);
+         has_dot |= (*cp == '.');
        }
       if (!has_dot)
        {
@@ -81,17 +81,27 @@ char *canonicalize_hostname(char *host)
 
          if (domain == NULL)
            {
+             struct utsname name;
+
              uname(&name);
              hp = gethostbyname(name.nodename);
-             cp = strchr(hp->h_name, '.');
-             if (cp)
-               domain = strdup(++cp);
+             if (hp)
+               {
+                 cp = strchr(hp->h_name, '.');
+                 if (cp)
+                   domain = strdup(++cp);
+                 if (!domain)
+                   domain = "";
+               }
              else
                domain = "";
            }
+         tbuf = malloc(strlen(host) + strlen(domain) + 2);
+         if (!tbuf)
+           return NULL;
          sprintf(tbuf, "%s.%s", host, domain);
          free(host);
-         host = strdup(tbuf);
+         host = tbuf;
        }
       return host;
     }

diff --git a/lib/fixname.c b/lib/fixname.c
index a10d06dc41e06418329afda3c23a792f239f0cf5..178f3feaef92195ebfdd816aa7c9551510509702 100644 (file)
--- a/lib/fixname.c
+++ b/lib/fixname.c
@@ -108,6 +108,8 @@ void LookForJrAndIII(char *nm, int *pends_jr, int *pends_sr, int *pends_ii,
     }
 }
 
+/* XXX no way to avoid possible buffer overrun here since we don't know
+   how long nm is and we're trying to make it one longer */
 void LookForSt(char *nm)               /* ST PIERRE, etc. */
 {
   char temp[256];

diff --git a/lib/mr_access.c b/lib/mr_access.c
index 8d120694f0274b4ed556b73d622bbc088486b378..711754a7fd274b7ab755b485123485a0e1b7ec9a 100644 (file)
--- a/lib/mr_access.c
+++ b/lib/mr_access.c
@@ -11,6 +11,7 @@
 #include <moira.h>
 #include "mr_private.h"
 
+#include <errno.h>
 #include <stdlib.h>
 #include <string.h>
 
@@ -26,6 +27,8 @@ int mr_access(char *name, int argc, char **argv)
   char **nargv = malloc(sizeof(char *) * (argc + 1));
   int status = 0;
 
+  if (!nargv)
+    return ENOMEM;
   nargv[0] = name;
   memcpy(nargv + 1, argv, sizeof(char *) * argc);
   status = mr_access_internal(argc + 1, nargv);

diff --git a/lib/mr_connect.c b/lib/mr_connect.c
index 388d3d8f7a4140d38ba19a70c41550aad9105c69..5fe7d8a512033ecf1ca4639c129c9f3d2a602ed3 100644 (file)
--- a/lib/mr_connect.c
+++ b/lib/mr_connect.c
@@ -31,7 +31,7 @@ static char *mr_server_host = 0;
 
 int mr_connect(char *server)
 {
-  char *p, **pp, sbuf[256];
+  char *p, **pp, *sbuf = NULL;
 
   if (!mr_inited)
     mr_init();
@@ -57,6 +57,9 @@ int mr_connect(char *server)
     {
       p = strchr(MOIRA_SERVER, ':');
       p++;
+      sbuf = malloc(strlen(server) + strlen(p) + 2);
+      if (!sbuf)
+       return ENOMEM;
       sprintf(sbuf, "%s:%s", server, p);
       server = sbuf;
     }
@@ -78,7 +81,9 @@ int mr_connect(char *server)
    * stash hostname for later use
    */
 
-  mr_server_host = strdup(server);
+  if (!sbuf)
+    sbuf = strdup(server);
+  mr_server_host = sbuf;
   if ((p = strchr(mr_server_host, ':')))
     *p = '\0';
   mr_server_host = canonicalize_hostname(mr_server_host);

diff --git a/lib/mr_ops.c b/lib/mr_ops.c
index e3a468a58daa11f2005eafafbdffe35dc7c70870..015d5449898df67ff544ca52113ba5cc591fe2b1 100644 (file)
--- a/lib/mr_ops.c
+++ b/lib/mr_ops.c
@@ -13,6 +13,8 @@
 #include <moira.h>
 #include "mr_private.h"
 
+#include <errno.h>
+#include <stdlib.h>
 #include <string.h>
 
 RCSID("$Header$");
@@ -53,7 +55,7 @@ int mr_motd(char **motd)
   mr_params param_st;
   struct mr_params *params = NULL;
   struct mr_params *reply = NULL;
-  static char buffer[1024];
+  static char *buffer = NULL;
 
   *motd = NULL;
   CHECK_CONNECTED;
@@ -71,7 +73,13 @@ int mr_motd(char **motd)
     {
       if (reply->mr_argc > 0)
        {
-         strncpy(buffer, reply->mr_argv[0], sizeof(buffer));
+         buffer = realloc(buffer, reply->mr_argl[0] + 1);
+         if (!buffer)
+           {
+             mr_disconnect();
+             return ENOMEM;
+           }
+         strcpy(buffer, reply->mr_argv[0]);
          *motd = buffer;
        }
       mr_destroy_reply(reply);

diff --git a/lib/mr_query.c b/lib/mr_query.c
index 5b3b0e3a2c105ca47791da709817b7bbd7b34b03..af22ef891f5e265fb86e42151a94b92ffdc29d17 100644 (file)
--- a/lib/mr_query.c
+++ b/lib/mr_query.c
@@ -12,6 +12,7 @@
 #include <moira.h>
 #include "mr_private.h"
 
+#include <errno.h>
 #include <stdlib.h>
 #include <string.h>
 
@@ -34,6 +35,8 @@ int mr_query(char *name, int argc, char **argv,
   char **nargv = malloc(sizeof(char *) * (argc + 1));
   int status = 0;
 
+  if (!nargv)
+    return ENOMEM;
   nargv[0] = name;
   memcpy(nargv + 1, argv, sizeof(char *) * argc);
   status = mr_query_internal(argc + 1, nargv, callproc, callarg);