public inbox for b.a.t.m.a.n@lists.open-mesh.org
 help / color / mirror / Atom feed
* [B.A.T.M.A.N.] [PATCH] batman-adv: Check size information when reassembling fragments
@ 2014-11-25 18:06 Sven Eckelmann
  2014-11-25 18:11 ` Christian Huldt
  2014-11-25 18:39 ` Martin Hundebøll
  0 siblings, 2 replies; 14+ messages in thread
From: Sven Eckelmann @ 2014-11-25 18:06 UTC (permalink / raw)
  To: b.a.t.m.a.n; +Cc: Sven Eckelmann

The fragmentation code doesn't check if the the total_size information of the
packets inside one chain is consistent. This allows an attacker to inject
packets belonging to the same fragmentation sequence number with different
total_size. This can cause a crash when these are assembled because the
total_size information is only parsed from the first packet in
batadv_frag_merge_packets but the queueing function always uses the total_size
of the latest packet.

Assume two packets with the size x and y.

1. first packet is sent with a size x and the total_size x+y' (y' < y)
2. second packet is sent with a size y and the total_size x+y

The fragmentation code would try to assemble the two packets because the
accumulated packets have a combined size of x+y and the second packet had the
total_size of x+y.

The fragmentation assembling code only took the information from the first
packet with the total_size x+y' and create a packet with enough space for x+y'
bytes. But the second packet cannot be copied inside the prepared free space
because it is y-y' bytes larger than the remaining space.

Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
This is only build tested. I've never spend time in creation of these packets
to verify my claim.

 fragmentation.c | 7 +++++--
 types.h         | 2 ++
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/fragmentation.c b/fragmentation.c
index 362e91a..3a19d4d 100644
--- a/fragmentation.c
+++ b/fragmentation.c
@@ -161,6 +161,7 @@ static bool batadv_frag_insert_packet(struct batadv_orig_node *orig_node,
 		hlist_add_head(&frag_entry_new->list, &chain->head);
 		chain->size = skb->len - hdr_size;
 		chain->timestamp = jiffies;
+		chain->total_size = ntohs(frag_packet->total_size);
 		ret = true;
 		goto out;
 	}
@@ -195,9 +196,11 @@ static bool batadv_frag_insert_packet(struct batadv_orig_node *orig_node,
 
 out:
 	if (chain->size > batadv_frag_size_limit() ||
-	    ntohs(frag_packet->total_size) > batadv_frag_size_limit()) {
+	    chain->total_size != ntohs(frag_packet->total_size) ||
+	    chain->total_size > batadv_frag_size_limit()) {
 		/* Clear chain if total size of either the list or the packet
-		 * exceeds the maximum size of one merged packet.
+		 * exceeds the maximum size of one merged packet. Don't allow
+		 * packets to have different total_size.
 		 */
 		batadv_frag_clear_chain(&chain->head);
 		chain->size = 0;
diff --git a/types.h b/types.h
index 462a70c..c4d7d24 100644
--- a/types.h
+++ b/types.h
@@ -132,6 +132,7 @@ struct batadv_orig_ifinfo {
  * @timestamp: time (jiffie) of last received fragment
  * @seqno: sequence number of the fragments in the list
  * @size: accumulated size of packets in list
+ * @total_size: expected size of the assembled packet
  */
 struct batadv_frag_table_entry {
 	struct hlist_head head;
@@ -139,6 +140,7 @@ struct batadv_frag_table_entry {
 	unsigned long timestamp;
 	uint16_t seqno;
 	uint16_t size;
+	uint16_t total_size;
 };
 
 /**
-- 
2.1.3


^ permalink raw reply related	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2014-11-30 20:04 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-25 18:06 [B.A.T.M.A.N.] [PATCH] batman-adv: Check size information when reassembling fragments Sven Eckelmann
2014-11-25 18:11 ` Christian Huldt
2014-11-25 18:39 ` Martin Hundebøll
2014-11-25 21:16   ` Philipp Psurek
2014-11-30 10:36   ` Philipp Psurek
2014-11-30 11:20     ` Philipp Psurek
2014-11-30 12:26       ` Martin Hundebøll
2014-11-30 13:35         ` Philipp Psurek
2014-11-30 13:40           ` Martin Hundebøll
2014-11-30 14:07             ` Philipp Psurek
2014-11-30 17:04               ` Philipp Psurek
2014-11-30 18:11                 ` Philipp Psurek
2014-11-30 18:35                   ` Philipp Psurek
2014-11-30 20:04                   ` Philipp Psurek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox