strict strtol checking. text2sockaddr/sockaddr2text implementations

for systems without get{addr,name}info calls.  Some style police.

3 files changed, 114 insertions, 41 deletions

diff --git a/sbin/isakmpd/isakmpd.c b/sbin/isakmpd/isakmpd.c
index bfacd5b9683..18f77bba9fc 100644
--- a/sbin/isakmpd/isakmpd.c
+++ b/sbin/isakmpd/isakmpd.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: isakmpd.c,v 1.31 2001/06/29 19:41:43 ho Exp $	*/
+/*	$OpenBSD: isakmpd.c,v 1.32 2001/07/01 19:59:13 niklas Exp $	*/
 /*	$EOM: isakmpd.c,v 1.54 2000/10/05 09:28:22 niklas Exp $	*/
 
 /*
@@ -115,6 +115,7 @@ static void
 parse_args (int argc, char *argv[])
 {
   int ch;
+  char *ep;
 #ifdef USE_DEBUG
   int cls, level;
   int do_packetlog = 0;
@@ -138,7 +139,7 @@ parse_args (int argc, char *argv[])
 	      {
 		  for (cls = 0; cls < LOG_ENDCLASS; cls++)
 		    log_debug_cmd (cls, level);
-	      }  
+	      }
 	    else
 	      log_print ("parse_args: -D argument unparseable: %s", optarg);
 	}
@@ -178,7 +179,10 @@ parse_args (int argc, char *argv[])
 #endif /* USE_DEBUG */
 
     case 'r':
-      srandom (strtoul (optarg, 0, 0));
+      seed = strtoul (optarg, &ep, 0);
+      srandom (seed);
+      if (*ep != '\0')
+	log_fatal ("parse_args: invalid numeric arg to -r (%s)", optarg);
       regrand = 1;
       break;
 
@@ -206,8 +210,8 @@ reinit (void)
 {
   log_print ("SIGHUP recieved, reinitializing daemon.");
 
-  /* 
-   * XXX Remove all(/some?) pending exchange timers? - they may not be 
+  /*
+   * XXX Remove all(/some?) pending exchange timers? - they may not be
    *     possible to complete after we've re-read the config file.
    *     User-initiated SIGHUP's maybe "authorizes" a wait until
    *     next connection-check.
@@ -216,7 +220,7 @@ reinit (void)
 
   /* Reinitialize PRNG if we are in deterministic mode.  */
   if (regrand)
-    srandom (strtoul (optarg, 0, 0));
+    srandom (seed);
 
   /* Reread config file.  */
   conf_reinit ();
@@ -240,7 +244,7 @@ reinit (void)
 
   /*
    * XXX Rescan interfaces.
-   *   transport_reinit (); 
+   *   transport_reinit ();
    *   udp_reinit ();
    */
 
@@ -266,7 +270,7 @@ report (void)
 {
   FILE *report, *old;
   mode_t old_umask;
-	
+
   old_umask = umask (S_IRWXG | S_IRWXO);
   report = fopen (report_file, "w");
   umask (old_umask);
@@ -300,7 +304,7 @@ rehash_timers (void)
 #if 0
   /* XXX - not yet */
   log_print ("SIGUSR2 received, rehasing soft expiration timers.");
-  
+
   timer_rehash_timers ();
 #endif
 
@@ -367,7 +371,7 @@ main (int argc, char *argv[])
   if (pcap_file != 0)
     log_packet_init (pcap_file);
 #endif
-  
+
   /* Allocate the file descriptor sets just big enough.  */
   n = getdtablesize ();
   mask_size = howmany (n, NFDBITS) * sizeof (fd_mask);
@@ -391,7 +395,7 @@ main (int argc, char *argv[])
       /* and if someone sent SIGUSR2, do a timer rehash.  */
       if (sigusr2ed)
 	rehash_timers ();
-      
+
       /* Setup the descriptors to look for incoming messages at.  */
       memset (rfds, 0, mask_size);
       n = transport_fd_set (rfds);
diff --git a/sbin/isakmpd/util.c b/sbin/isakmpd/util.c
index 2e83989dc9f..bf814f6b8c0 100644
--- a/sbin/isakmpd/util.c
+++ b/sbin/isakmpd/util.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: util.c,v 1.19 2001/07/01 14:23:30 ho Exp $	*/
+/*	$OpenBSD: util.c,v 1.20 2001/07/01 19:59:13 niklas Exp $	*/
 /*	$EOM: util.c,v 1.23 2000/11/23 12:22:08 niklas Exp $	*/
 
 /*
@@ -40,6 +40,7 @@
 #include <sys/stat.h>
 #include <netinet/in.h>
 #include <arpa/inet.h>
+#include <limits.h>
 #include <netdb.h>
 #include <stdlib.h>
 #include <string.h>
@@ -67,6 +68,11 @@ int allow_name_lookups = 0;
 int regrand = 0;
 
 /*
+ * If in regression-test mode, this is the seed used.
+ */
+unsigned long seed;
+
+/*
  * XXX These might be turned into inlines or macros, maybe even
  * machine-dependent ones, for performance reasons.
  */
@@ -265,7 +271,42 @@ text2sockaddr (char *address, char *port, struct sockaddr **sa)
   freeaddrinfo (ai);
   return 0;
 #else
-  return -1;
+  int af = strchr (address, ':') != NULL ? AF_INET6 : AF_INET;
+  size_t sz = af == AF_INET
+    ? sizeof (struct sockaddr_in) : sizeof (struct sockaddr_in6);
+  long lport;
+  struct servent *sp;
+  char *ep;
+
+  *sa = calloc (1, sz);
+  if (!*sa)
+    return -1;
+
+  (*sa)->sa_len = sz;
+  (*sa)->sa_family = af;
+  if (inet_pton (af, address, sockaddr_data (*sa)) != 1)
+    {
+      free (*sa);
+      return -1;
+    }
+  sp = getservbyname (port, "udp");
+  if (!sp)
+    {
+      lport = strtol (port, &ep, 10);
+      if (ep == port || lport < 0 || lport > USHRT_MAX)
+	{
+	  free (*sa);
+	  return -1;
+	}
+      lport = htons (lport);
+    }
+  else
+    lport = sp->s_port;
+  if ((*sa)->sa_family == AF_INET)
+    ((struct sockaddr_in *)*sa)->sin_port = lport;
+  else
+    ((struct sockaddr_in6 *)*sa)->sin6_port = lport;
+  return 0;
 #endif
 }
 
@@ -277,32 +318,39 @@ int
 sockaddr2text (struct sockaddr *sa, char **address, int zflag)
 {
   char buf[NI_MAXHOST];
-  char *token, *bstart, *p;
+  char *token, *bstart, *p, *ep;
   int addrlen, c_pre = 0, c_post = 0;
+  long val;
 
 #ifdef HAVE_GETNAMEINFO
   if (getnameinfo (sa, sa->sa_len, buf, sizeof buf, 0, 0,
 		   allow_name_lookups ? 0 : NI_NUMERICHOST))
     return -1;
 #else
-  if (sa->sa_family == AF_INET)
+  switch (sa->sa_family)
     {
-      strncpy (buf, inet_ntoa (((struct sockaddr_in *)sa)->sin_addr), 
-	       NI_MAXHOST - 1);
+    case AF_INET:
+    case AF_INET6:
+      if (inet_ntop (sa->sa_family, sa->sa_data, buf, NI_MAXHOST - 1) == NULL)
+	{
+	  log_error ("sockaddr2text: inet_ntop (%d, %p, %p, %d) failed",
+		     sa->sa_family, sa->sa_data, buf, NI_MAXHOST - 1);
+	  return -1;
+	}
       buf[NI_MAXHOST - 1] = '\0';
-    }
-  else
-    {
+      break;
+
+    default:
       log_print ("sockaddr2text: unsupported protocol family %d\n",
 		  sa->sa_family);
-      strcpy (buf, "<error>");
+      return -1;
     }
 #endif
 
   if (zflag == 0)
     {
       *address = malloc (strlen (buf) + 1);
-      if (*address == NULL)
+      if (!*address)
 	return -1;
       strcpy (*address, buf);
     }
@@ -314,8 +362,9 @@ sockaddr2text (struct sockaddr *sa, char **address, int zflag)
 	*address = malloc (addrlen);
 	if (!*address)
 	  return -1;
-	buf[addrlen] = '\0'; /* Terminate */
-	bstart = buf; **address = '\0';
+	buf[addrlen] = '\0';
+	bstart = buf;
+	**address = '\0';
 	while ((token = strsep (&bstart, ".")) != NULL)
 	  {
 	    if (strlen (*address) > 12)
@@ -323,24 +372,35 @@ sockaddr2text (struct sockaddr *sa, char **address, int zflag)
 		free (*address);
 		return -1;
 	      }
-	    sprintf (*address + strlen (*address), "%03ld",
-		     strtol (token, NULL, 10));
+	    val = strtol (token, &ep, 10);
+	    if (ep == token || val < 0 || val > UCHAR_MAX)
+	      {
+		free (*address);
+		return -1;
+	      }
+	    sprintf (*address + strlen (*address), "%03ld", val);
 	    if (bstart)
 	      strcat (*address + strlen (*address), ".");
 	  }
 	break;
+
       case AF_INET6:
+	/*
+	 * XXX In the algorithm below there are some magic numbers we
+	 * probably could give explaining names.
+	 */
 	addrlen = sizeof "0000:0000:0000:0000:0000:0000:0000:0000";
 	*address = malloc (addrlen);
 	if (!*address)
 	  return -1;
-	bstart = buf; **address = '\0';
-	buf[addrlen] = '\0'; /* Terminate */
+	bstart = buf;
+	**address = '\0';
+	buf[addrlen] = '\0';
 	while ((token = strsep (&bstart, ":")) != NULL)
 	  {
 	    if (strlen (token) == 0)
 	      {
-		/* 
+		/*
 		 * Encountered a '::'. Fill out the string.
 		 * XXX Isn't there a library function for this somewhere?
 		 */
@@ -367,13 +427,19 @@ sockaddr2text (struct sockaddr *sa, char **address, int zflag)
 		    free (*address);
 		    return -1;
 		  }
-		sprintf (*address + strlen (*address), "%04lx", 
-			 strtol (token, NULL, 16));
+		val = strtol (token, &ep, 10);
+		if (ep == token || val < 0 || val > USHRT_MAX)
+		  {
+		    free (*address);
+		    return -1;
+		  }
+		sprintf (*address + strlen (*address), "%04lx", val);
 		if (bstart)
 		  strcat (*address + strlen (*address), ":");
 	      }
 	  }
 	break;
+
       default:
 	strcpy (*address, "<error>");
       }
@@ -383,7 +449,7 @@ sockaddr2text (struct sockaddr *sa, char **address, int zflag)
 
 /*
  * sockaddr_len and sockaddr_data return the relevant sockaddr info depending
- * on address family. Useful to keep other code shorter(/clearer?).
+ * on address family.  Useful to keep other code shorter(/clearer?).
  */
 int
 sockaddr_len (struct sockaddr *sa)
@@ -394,8 +460,8 @@ sockaddr_len (struct sockaddr *sa)
       return sizeof ((struct sockaddr_in6 *)sa)->sin6_addr.s6_addr;
     case AF_INET:
       return sizeof ((struct sockaddr_in *)sa)->sin_addr.s_addr;
-    default: 
-      log_print ("sockaddr_len: unsupported protocol family %d", 
+    default:
+      log_print ("sockaddr_len: unsupported protocol family %d",
 		 sa->sa_family);
       return 0;
     }
@@ -411,7 +477,7 @@ sockaddr_data (struct sockaddr *sa)
     case AF_INET:
       return (u_int8_t *)&((struct sockaddr_in *)sa)->sin_addr.s_addr;
     default:
-      return 0; /* XXX */
+      return 0;
     }
 }
 
@@ -434,9 +500,9 @@ util_ntoa (char **buf, int af, u_int8_t *addr)
     case AF_INET:
       sfrom->sa_len = sizeof (struct sockaddr_in);
       memcpy (&ip4_buf, addr, sizeof (struct in_addr));
-/*      ((struct sockaddr_in *)sfrom)->sin_addr.s_addr = htonl (ip4_buf);*/
       ((struct sockaddr_in *)sfrom)->sin_addr.s_addr = ip4_buf;
       break;
+
     case AF_INET6:
       sfrom->sa_len = sizeof (struct sockaddr_in6);
       memcpy (sockaddr_data (sfrom), addr, sizeof (struct in6_addr));
@@ -445,7 +511,8 @@ util_ntoa (char **buf, int af, u_int8_t *addr)
 
   if (sockaddr2text (sfrom, buf, 0))
     {
-      log_error ("util_ntoa: sockaddr2text () failed");
+      log_print ("util_ntoa: "
+		 "could not make printable address out of sockaddr %p", sfrom);
       *buf = 0;
     }
 }
@@ -453,13 +520,13 @@ util_ntoa (char **buf, int af, u_int8_t *addr)
 /*
  * Perform sanity check on files containing secret information.
  * Returns -1 on failure, 0 otherwise.
- * Also, if *file_size != NULL, store file size here.
+ * Also, if FILE_SIZE is a not a null pointer, store file size here.
  */
 int
 check_file_secrecy (char *name, off_t *file_size)
 {
   struct stat st;
-  
+
   if (stat (name, &st) == -1)
     {
       log_error ("check_file_secrecy: stat (\"%s\") failed", name);
@@ -479,7 +546,7 @@ check_file_secrecy (char *name, off_t *file_size)
       errno = EPERM;
       return -1;
     }
-  
+
   if (file_size)
     *file_size = st.st_size;
 
diff --git a/sbin/isakmpd/util.h b/sbin/isakmpd/util.h
index 8e7e4defbb2..2b4dedc8749 100644
--- a/sbin/isakmpd/util.h
+++ b/sbin/isakmpd/util.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: util.h,v 1.9 2001/06/29 04:12:01 ho Exp $	*/
+/*	$OpenBSD: util.h,v 1.10 2001/07/01 19:59:14 niklas Exp $	*/
 /*	$EOM: util.h,v 1.10 2000/10/24 13:33:39 niklas Exp $	*/
 
 /*
@@ -44,8 +44,10 @@
 
 extern int allow_name_lookups;
 extern int regrand;
+extern unsigned long seed;
 
 struct message;
+struct sockaddr;
 
 extern u_int16_t decode_16 (u_int8_t *);
 extern u_int32_t decode_32 (u_int8_t *);