[prev in list] [next in list] [prev in thread] [next in thread] 

List:       ipsec-tools-devel
Subject:    Re: [Ipsec-tools-devel] [Ipsec-tools-core] Potential Vulnerability Discovered in IPsec-Tools
From:       Rainer Weikusat <rweikusat () mobileactivedefense ! com>
Date:       2016-10-18 20:45:38
Message-ID: 878ttl4c99.fsf () doppelsaurus ! mobileactivedefense ! com
[Download RAW message or body]

Rainer Weikusat <rweikusat@mobileactivedefense.com> writes:

[...]


> +                       current = next;
> +                       if (current->frag_num == item->frag_num) {
> +                               plog(LLV_DEBUG, LOCATION, NULL, "duplicate fragment %d\n",
> +                                    item->frag_num);
> +
> +                               free(item);
> +                               return 0;
> +                       }

This leaks memory in both early exits as the data is in a dynamically
allocated buffer[*].

[*] It also calls free instead of racoon_free. This doesn't really
matter because the only difference is that the code could be compiled
with the Boehm-GC and I doubt anyone uses that (plain malloc is also
used in other places).

---
diff -rNu ipsec-tools-0.8.0/src/racoon/isakmp_frag.c patched/src/racoon/isakmp_frag.c
--- ipsec-tools-0.8.0/src/racoon/isakmp_frag.c	2009-04-22 12:24:20.000000000 +0100
+++ patched/src/racoon/isakmp_frag.c	2016-10-18 21:37:12.033038458 +0100
@@ -231,14 +231,35 @@
 	if (iph1->frag_chain == NULL) {
 		iph1->frag_chain = item;
 	} else {
-		struct isakmp_frag_item *current;
+		struct isakmp_frag_item *current, *next;
 
-		current = iph1->frag_chain;
-		while (current->frag_next) {
-			if (current->frag_last)
-				last_frag = item->frag_num;
-			current = current->frag_next;
-		}
+		next = iph1->frag_chain;
+		do {
+			current = next;
+			if (current->frag_num == item->frag_num) {
+				plog(LLV_DEBUG, LOCATION, NULL, "duplicate fragment %d\n",
+				     item->frag_num);
+				
+				racoon_free(item);
+				vfree(buf);
+				
+				return 0;
+			}
+
+			if (current->last_frag) {
+				if (item->last_frag) {
+					plog(LLV_WARNING, LOCATION, NULL, "multiple last fragments received\n");
+					
+					racoon_free(item);
+					vfree(buf);
+					
+					return -1;
+				}
+
+				last_frag = current->frag_num;
+			}
+		} while ((next = next->next));
+		
 		current->frag_next = item;
 	}
 

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Ipsec-tools-devel mailing list
Ipsec-tools-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ipsec-tools-devel
[prev in list] [next in list] [prev in thread] [next in thread]