Re: IPV4_RANGE causes isakmpd crash

[Hakan Olsson <ho@crt.se>]

On Tue, 10 Jun 2003, Eric Boudrand wrote:

> Isakmpd can crash during Phase 2 if a remote host uses ID like
> ID_IPV4_RANGE, ID_IPV6_RANGE, ID_DER_ASN1_DN, ID_DER_ASN1_GN et ID_KEY_ID
> (read 4.6.2.1 in RFC 2407).
>
> This occurs in ipsec_set_network function in ipsec.c file. There is a 4
> switch instructions in which these case are not supported. So when the
> execution get out of the switch, some pointers are not filled. Then it
> crashes.
>
> ID_IPV4_RANGE is used for example by ZyWall 10.
>
> Here is the patch for isakmpd.
> ---------- cut here -------------
...

Hi,

thanks for pointing this out. I just committed a fix which is effectively
the same as your suggested patch, although I took the opportunity to fix a
few nits along the way.

Here's the diff (or get it from CVS in a couple of hours):

Index: ipsec.c
===================================================================
RCS file: /cvs/src/sbin/isakmpd/ipsec.c,v
retrieving revision 1.76
diff -u -u -r1.76 ipsec.c
--- ipsec.c	4 Jun 2003 07:31:16 -0000	1.76
+++ ipsec.c	10 Jun 2003 12:19:47 -0000
@@ -331,7 +331,7 @@
 		  /* Initiator is source, responder is destination.  */
 		  if (ipsec_set_network (ie->id_ci, ie->id_cr, isa))
 		    {
-		      log_error ("ipsec_finalize_exchange: "
+		      log_print ("ipsec_finalize_exchange: "
 				 "ipsec_set_network failed");
 		      return;
 		    }
@@ -341,7 +341,7 @@
 		  /* Responder is source, initiator is destination.  */
 		  if (ipsec_set_network (ie->id_cr, ie->id_ci, isa))
 		    {
-		      log_error ("ipsec_finalize_exchange: "
+		      log_print ("ipsec_finalize_exchange: "
 				 "ipsec_set_network failed");
 		      return;
 		    }
@@ -414,6 +414,7 @@
 ipsec_set_network (u_int8_t *src_id, u_int8_t *dst_id, struct ipsec_sa *isa)
 {
   int id;
+  char *v;

   /* Set source address/mask.  */
   id = GET_ISAKMP_ID_TYPE (src_id);
@@ -424,7 +425,7 @@
       isa->src_net =
 	(struct sockaddr *)calloc (1, sizeof (struct sockaddr_in));
       if (!isa->src_net)
-	return -1;
+	goto memfail;
       isa->src_net->sa_family = AF_INET;
 #ifndef USE_OLD_SOCKADDR
       isa->src_net->sa_len = sizeof (struct sockaddr_in);
@@ -433,7 +434,7 @@
       isa->src_mask =
 	(struct sockaddr *)calloc (1, sizeof (struct sockaddr_in));
       if (!isa->src_mask)
-	return -1;
+	goto memfail;
       isa->src_mask->sa_family = AF_INET;
 #ifndef USE_OLD_SOCKADDR
       isa->src_mask->sa_len = sizeof (struct sockaddr_in);
@@ -445,7 +446,7 @@
       isa->src_net =
 	(struct sockaddr *)calloc (1, sizeof (struct sockaddr_in6));
       if (!isa->src_net)
-	return -1;
+	goto memfail;
       isa->src_net->sa_family = AF_INET6;
 #ifndef USE_OLD_SOCKADDR
       isa->src_net->sa_len = sizeof (struct sockaddr_in6);
@@ -454,12 +455,23 @@
       isa->src_mask =
 	(struct sockaddr *)calloc (1, sizeof (struct sockaddr_in6));
       if (!isa->src_mask)
-	return -1;
+	goto memfail;
       isa->src_mask->sa_family = AF_INET6;
 #ifndef USE_OLD_SOCKADDR
       isa->src_mask->sa_len = sizeof (struct sockaddr_in6);
 #endif
       break;
+
+    case IPSEC_ID_IPV4_RANGE:
+    case IPSEC_ID_IPV6_RANGE:
+    case IPSEC_ID_DER_ASN1_DN:
+    case IPSEC_ID_DER_ASN1_GN:
+    case IPSEC_ID_KEY_ID:
+    default:
+      v = constant_lookup (ipsec_id_cst, id);
+      log_print ("ipsec_set_network: ID type %d (%s) not supported",
+		 id, v ? v : "<unknown>");
+      return -1;
     }

   /* Net */
@@ -494,7 +506,7 @@
       isa->dst_net =
 	(struct sockaddr *)calloc (1, sizeof (struct sockaddr_in));
       if (!isa->dst_net)
-	return -1;
+	goto memfail;
       isa->dst_net->sa_family = AF_INET;
 #ifndef USE_OLD_SOCKADDR
       isa->dst_net->sa_len = sizeof (struct sockaddr_in);
@@ -503,7 +515,7 @@
       isa->dst_mask =
 	(struct sockaddr *)calloc (1, sizeof (struct sockaddr_in));
       if (!isa->dst_mask)
-	return -1;
+	goto memfail;
       isa->dst_mask->sa_family = AF_INET;
 #ifndef USE_OLD_SOCKADDR
       isa->dst_mask->sa_len = sizeof (struct sockaddr_in);
@@ -515,7 +527,7 @@
       isa->dst_net =
 	(struct sockaddr *)calloc (1, sizeof (struct sockaddr_in6));
       if (!isa->dst_net)
-	return -1;
+	goto memfail;
       isa->dst_net->sa_family = AF_INET6;
 #ifndef USE_OLD_SOCKADDR
       isa->dst_net->sa_len = sizeof (struct sockaddr_in6);
@@ -524,7 +536,7 @@
       isa->dst_mask =
 	(struct sockaddr *)calloc (1, sizeof (struct sockaddr_in6));
       if (!isa->dst_mask)
-	return -1;
+	goto memfail;
       isa->dst_mask->sa_family = AF_INET6;
 #ifndef USE_OLD_SOCKADDR
       isa->dst_mask->sa_len = sizeof (struct sockaddr_in6);
@@ -557,6 +569,10 @@
   memcpy (&isa->dport, dst_id + ISAKMP_ID_DOI_DATA_OFF + IPSEC_ID_PORT_OFF,
 	  IPSEC_ID_PORT_LEN);
   return 0;
+
+ memfail:
+  log_error ("ipsec_set_network: calloc () failed");
+  return -1;
 }

 /* Free the DOI-specific exchange data pointed to by VIE.  */


//H

--
Håkan Olsson <ho@crt.se>        (+46) 708 437 337     Carlstedt Research
Unix, Networking, Security      (+46) 31 701 4264        & Technology AB