From 98f0c101b93c70e297c317369f3a05bb26c199f1 Mon Sep 17 00:00:00 2001
From: csoler <csoler@users.sourceforge.net>
Date: Mon, 11 Jan 2016 19:26:54 -0500
Subject: [PATCH] fixed potential integer overflow / Out of bounds read in
 GRouterItems.cc

---
 libretroshare/src/grouter/grouteritems.cc | 49 ++++++++++++++---------
 1 file changed, 30 insertions(+), 19 deletions(-)

diff --git a/libretroshare/src/grouter/grouteritems.cc b/libretroshare/src/grouter/grouteritems.cc
index b67ed71e6..17b3cb2b4 100644
--- a/libretroshare/src/grouter/grouteritems.cc
+++ b/libretroshare/src/grouter/grouteritems.cc
@@ -66,6 +66,12 @@ RsGRouterTransactionChunkItem *RsGRouterSerialiser::deserialise_RsGRouterTransac
     uint32_t rssize = getRsItemSize(data);
     bool ok = true ;
 
+    if(tlvsize < rssize)
+    {
+        std::cerr << __PRETTY_FUNCTION__ << ": wrong encoding of item size. Serialisation error!" << std::endl;
+        return NULL ;
+    }
+        
     RsGRouterTransactionChunkItem *item = new RsGRouterTransactionChunkItem() ;
 
     /* add mandatory parts first */
@@ -74,15 +80,15 @@ RsGRouterTransactionChunkItem *RsGRouterSerialiser::deserialise_RsGRouterTransac
     ok &= getRawUInt32(data, tlvsize, &offset, &item->chunk_size);
     ok &= getRawUInt32(data, tlvsize, &offset, &item->total_size);
 
-    if( NULL == (item->chunk_data = (uint8_t*)malloc(item->chunk_size)))
+    if(item->chunk_size > rssize || offset > rssize - item->chunk_size) // better than if(item->chunk_size + offset > rssize)
     {
-        std::cerr << __PRETTY_FUNCTION__ << ": Cannot allocate memory for chunk " << item->chunk_size << std::endl;
+        std::cerr << __PRETTY_FUNCTION__ << ": Cannot read beyond item size. Serialisation error!" << std::endl;
 	delete item;
         return NULL ;
     }
-    if(item->chunk_size + offset > rssize)
+    if( NULL == (item->chunk_data = (uint8_t*)malloc(item->chunk_size)))
     {
-        std::cerr << __PRETTY_FUNCTION__ << ": Cannot read beyond item size. Serialisation error!" << std::endl;
+        std::cerr << __PRETTY_FUNCTION__ << ": Cannot allocate memory for chunk " << item->chunk_size << std::endl;
 	delete item;
         return NULL ;
     }
@@ -125,36 +131,41 @@ RsGRouterGenericDataItem *RsGRouterSerialiser::deserialise_RsGRouterGenericDataI
 	uint32_t rssize = getRsItemSize(data);
 	bool ok = true ;
 
+	if(pktsize < rssize)
+	{
+		std::cerr << __PRETTY_FUNCTION__ << ": wrong encoding of item size. Serialisation error!" << std::endl;
+		return NULL ;
+	}
 	RsGRouterGenericDataItem *item = new RsGRouterGenericDataItem() ;
 
 	ok &= getRawUInt64(data, pktsize, &offset, &item->routing_id); 	
 	ok &= item->destination_key.deserialise(data, pktsize, offset) ;
-    ok &= getRawUInt32(data, pktsize, &offset, &item->service_id);
-    ok &= getRawUInt32(data, pktsize, &offset, &item->data_size);
+	ok &= getRawUInt32(data, pktsize, &offset, &item->service_id);
+	ok &= getRawUInt32(data, pktsize, &offset, &item->data_size);
 
-    if( NULL == (item->data_bytes = (uint8_t*)malloc(item->data_size)))
+    	if(item->data_size > rssize || offset > rssize - item->data_size) // better than if(item->data_size + offset > rssize)
+	{
+		std::cerr << __PRETTY_FUNCTION__ << ": Cannot read beyond item size. Serialisation error!" << std::endl;
+		delete item;
+		return NULL ;
+	}
+
+	if( NULL == (item->data_bytes = (uint8_t*)malloc(item->data_size)))
 	{
 		std::cerr << __PRETTY_FUNCTION__ << ": Cannot allocate memory for chunk " << item->data_size << std::endl;
 		delete item;
 		return NULL ;
 	}
 
-    if(item->data_size + offset > rssize)
-    {
-        std::cerr << __PRETTY_FUNCTION__ << ": Cannot read beyond item size. Serialisation error!" << std::endl;
-	delete item;
-        return NULL ;
-    }
-
-    memcpy(item->data_bytes,&((uint8_t*)data)[offset],item->data_size) ;
+	memcpy(item->data_bytes,&((uint8_t*)data)[offset],item->data_size) ;
 	offset += item->data_size ;
 
-    ok &= item->signature.GetTlv(data, pktsize, &offset) ;
+	ok &= item->signature.GetTlv(data, pktsize, &offset) ;
 
-    ok &= getRawUInt32(data, pktsize, &offset, &item->randomized_distance);
-    ok &= getRawUInt32(data, pktsize, &offset, &item->flags);
+	ok &= getRawUInt32(data, pktsize, &offset, &item->randomized_distance);
+	ok &= getRawUInt32(data, pktsize, &offset, &item->flags);
 
-    if (offset != rssize || !ok)
+	if (offset != rssize || !ok)
 	{
 		std::cerr << __PRETTY_FUNCTION__ << ": error while deserialising! Item will be dropped." << std::endl;
 		delete item;