Fixed more buffer overflows with small packets

8 files changed, 58 insertions, 55 deletions

diff --git a/src/lib/protocols/dcerpc.c b/src/lib/protocols/dcerpc.c
index ec96d1287..7be8ac027 100644
--- a/src/lib/protocols/dcerpc.c
+++ b/src/lib/protocols/dcerpc.c
@@ -36,13 +36,11 @@ void ndpi_search_dcerpc(struct ndpi_detection_module_struct *ndpi_struct, struct
 {
   struct ndpi_packet_struct *packet = &flow->packet;
 
-	u_int16_t len_packet = (packet->payload[9]<<8) | packet->payload[8];
-
   if((packet->tcp != NULL)
      && (packet->payload_packet_len >= 64)
      && (packet->payload[0] == 0x05) /* version 5 */
      && (packet->payload[2] < 16) /* Packet type */
-		 && (len_packet == packet->payload_packet_len) /* Packet Length */
+		 && (((packet->payload[9]<<8) | packet->payload[8]) == packet->payload_packet_len) /* Packet Length */
      ) {
     NDPI_LOG(NDPI_PROTOCOL_DCERPC, ndpi_struct, NDPI_LOG_DEBUG, "DCERPC match\n");
     ndpi_int_dcerpc_add_connection(ndpi_struct, flow);
diff --git a/src/lib/protocols/gnutella.c b/src/lib/protocols/gnutella.c
index 09d4d0852..e45096391 100644
--- a/src/lib/protocols/gnutella.c
+++ b/src/lib/protocols/gnutella.c
@@ -294,7 +294,7 @@ void ndpi_search_gnutella(struct ndpi_detection_module_struct *ndpi_struct, stru
       return;
     }
 
-    if (memcmp(packet->payload, "GND", 3) == 0) {
+    if (packet->payload_packet_len >= 3 && memcmp(packet->payload, "GND", 3) == 0) {
       if ((packet->payload_packet_len == 8 && (memcmp(&packet->payload[6], "\x01\x00", 2) == 0))
 	  || (packet->payload_packet_len == 11 && (memcmp(&packet->payload[6], "\x01\x01\x08\x50\x49", 5)
 						   == 0)) || (packet->payload_packet_len == 17
diff --git a/src/lib/protocols/h323.c b/src/lib/protocols/h323.c
index 1d503a747..7a94dabd5 100644
--- a/src/lib/protocols/h323.c
+++ b/src/lib/protocols/h323.c
@@ -27,7 +27,8 @@ void ndpi_search_h323(struct ndpi_detection_module_struct *ndpi_struct, struct n
     NDPI_LOG(NDPI_PROTOCOL_H323, ndpi_struct, NDPI_LOG_DEBUG, "calculated dport over tcp.\n");
 
     /* H323  */
-    if((packet->payload[0] == 0x03)
+    if(packet->payload_packet_len >= 3
+       && (packet->payload[0] == 0x03)
        && (packet->payload[1] == 0x00)
        && (packet->payload[2] == 0x00)) {
 	struct tpkt *t = (struct tpkt*)packet->payload;
@@ -63,7 +64,8 @@ void ndpi_search_h323(struct ndpi_detection_module_struct *ndpi_struct, struct n
     sport = ntohs(packet->udp->source), dport = ntohs(packet->udp->dest);
     NDPI_LOG(NDPI_PROTOCOL_H323, ndpi_struct, NDPI_LOG_DEBUG, "calculated dport over udp.\n");
 
-    if(packet->payload[0] == 0x80 && packet->payload[1] == 0x08 && (packet->payload[2] == 0xe7 || packet->payload[2] == 0x26) &&
+    if(packet->payload_packet_len >= 5 && packet->payload[0] == 0x80 && packet->payload[1] == 0x08 &&
+       (packet->payload[2] == 0xe7 || packet->payload[2] == 0x26) &&
        packet->payload[4] == 0x00 && packet->payload[5] == 0x00)
       {
 	NDPI_LOG(NDPI_PROTOCOL_H323, ndpi_struct, NDPI_LOG_DEBUG, "found H323 broadcast.\n");
diff --git a/src/lib/protocols/kakaotalk_voice.c b/src/lib/protocols/kakaotalk_voice.c
index c6972c7a1..368532c5d 100644
--- a/src/lib/protocols/kakaotalk_voice.c
+++ b/src/lib/protocols/kakaotalk_voice.c
@@ -33,7 +33,7 @@ void ndpi_search_kakaotalk_voice(struct ndpi_detection_module_struct *ndpi_struc
   
   if(packet->iph
      && packet->udp
-     && (packet->payload_packet_len > 0)
+     && (packet->payload_packet_len >= 4)
      ) {
     if((packet->payload[0] == 0x81)
        || (packet->payload[1] == 0xC8)
diff --git a/src/lib/protocols/radius.c b/src/lib/protocols/radius.c
index 625dc4108..308049522 100644
--- a/src/lib/protocols/radius.c
+++ b/src/lib/protocols/radius.c
@@ -37,12 +37,11 @@ static void ndpi_check_radius(struct ndpi_detection_module_struct *ndpi_struct,
 
   if(packet->udp != NULL) {
     struct radius_header *h = (struct radius_header*)packet->payload;
-    u_int len = ntohs(h->len);
 
     if((payload_len > sizeof(struct radius_header))
        && (h->code > 0)
        && (h->code <= 5)
-       && (len == payload_len)) {
+       && (ntohs(h->len) == payload_len)) {
       NDPI_LOG(NDPI_PROTOCOL_RADIUS, ndpi_struct, NDPI_LOG_DEBUG, "Found radius.\n");
       ndpi_set_detected_protocol(ndpi_struct, flow, NDPI_PROTOCOL_RADIUS, NDPI_PROTOCOL_UNKNOWN);
 
diff --git a/src/lib/protocols/rtcp.c b/src/lib/protocols/rtcp.c
index c8fc90953..be4aee669 100644
--- a/src/lib/protocols/rtcp.c
+++ b/src/lib/protocols/rtcp.c
@@ -38,7 +38,7 @@ void ndpi_search_rtcp(struct ndpi_detection_module_struct *ndpi_struct, struct n
     /* Let's check first the RTCP packet length */
     u_int16_t len, offset = 0, rtcp_section_len;
     
-    while(offset < packet->payload_packet_len) {
+    while(offset + 3 < packet->payload_packet_len) {
       len = packet->payload[2+offset] * 256 + packet->payload[2+offset+1];
       rtcp_section_len = (len + 1) * 4;
       
@@ -50,9 +50,10 @@ void ndpi_search_rtcp(struct ndpi_detection_module_struct *ndpi_struct, struct n
     
     sport = ntohs(packet->udp->source), dport = ntohs(packet->udp->dest);
     NDPI_LOG(NDPI_PROTOCOL_RTCP, ndpi_struct, NDPI_LOG_DEBUG, "calculating dport over udp.\n");
-    if(((packet->payload_packet_len >= 28 || packet->payload_packet_len <= 1200) &&
+    /* TODO changed a pair of length condition to the && from ||. Is it correct? */
+    if(((packet->payload_packet_len >= 28 && packet->payload_packet_len <= 1200) &&
 	((packet->payload[0] == 0x80) && ((packet->payload[1] == 0xc8) || (packet->payload[1] == 0xc9)) && (packet->payload[2] == 0x00)))
-       || (((packet->payload[0] == 0x81) && ((packet->payload[1] == 0xc8) || (packet->payload[1] == 0xc9))
+       || (packet->payload_packet_len >= 3 && ((packet->payload[0] == 0x81) && ((packet->payload[1] == 0xc8) || (packet->payload[1] == 0xc9))
 	    && (packet->payload[2] == 0x00)))) {
       NDPI_LOG(NDPI_PROTOCOL_RTCP, ndpi_struct, NDPI_LOG_DEBUG, "found rtcp.\n");
       ndpi_int_rtcp_add_connection(ndpi_struct, flow);
diff --git a/src/lib/protocols/spotify.c b/src/lib/protocols/spotify.c
index 274312163..e7dac5d66 100644
--- a/src/lib/protocols/spotify.c
+++ b/src/lib/protocols/spotify.c
@@ -54,7 +54,7 @@ static void ndpi_check_spotify(struct ndpi_detection_module_struct *ndpi_struct,
     }
   } else if(packet->tcp != NULL) {
 
-     if(packet->payload[0] == 0x00 && packet->payload[1] == 0x04 &&
+     if(payload_len >= 8 && packet->payload[0] == 0x00 && packet->payload[1] == 0x04 &&
        packet->payload[2] == 0x00 && packet->payload[3] == 0x00&&
        packet->payload[6] == 0x52 && packet->payload[7] == 0x0e &&
        packet->payload[8] == 0x50 ) {
diff --git a/src/lib/protocols/ssl.c b/src/lib/protocols/ssl.c
index 2269ae782..14deff7f9 100644
--- a/src/lib/protocols/ssl.c
+++ b/src/lib/protocols/ssl.c
@@ -223,64 +223,67 @@ int getSSLcertificate(struct ndpi_detection_module_struct *ndpi_struct,
 	}
       } else if(handshake_protocol == 0x01 /* Client Hello */) {
 	u_int offset, base_offset = 43;
-	u_int16_t session_id_len = packet->payload[base_offset];
+	if (base_offset + 2 <= packet->payload_packet_len)
+	{
+	  u_int16_t session_id_len = packet->payload[base_offset];
 
-	if((session_id_len+base_offset+2) <= total_len) {
-	  u_int16_t cypher_len =  packet->payload[session_id_len+base_offset+2] + (packet->payload[session_id_len+base_offset+1] << 8);
-	  offset = base_offset + session_id_len + cypher_len + 2;
+	  if((session_id_len+base_offset+2) <= total_len) {
+	    u_int16_t cypher_len =  packet->payload[session_id_len+base_offset+2] + (packet->payload[session_id_len+base_offset+1] << 8);
+	    offset = base_offset + session_id_len + cypher_len + 2;
 
-	  flow->l4.tcp.ssl_seen_client_cert = 1;
+	    flow->l4.tcp.ssl_seen_client_cert = 1;
 
-	  if(offset < total_len) {
-	    u_int16_t compression_len;
-	    u_int16_t extensions_len;
+	    if(offset < total_len) {
+	      u_int16_t compression_len;
+	      u_int16_t extensions_len;
 
-	    compression_len = packet->payload[offset+1];
-	    offset += compression_len + 3;
+	      compression_len = packet->payload[offset+1];
+	      offset += compression_len + 3;
 
-	    if(offset < total_len) {
-	      extensions_len = packet->payload[offset];
+	      if(offset < total_len) {
+		extensions_len = packet->payload[offset];
 
-	      if((extensions_len+offset) < total_len) {
-		u_int16_t extension_offset = 1; /* Move to the first extension */
+		if((extensions_len+offset) < total_len) {
+		  u_int16_t extension_offset = 1; /* Move to the first extension */
 
-		while(extension_offset < extensions_len) {
-		  u_int16_t extension_id, extension_len;
+		  while(extension_offset < extensions_len) {
+		    u_int16_t extension_id, extension_len;
 
-		  memcpy(&extension_id, &packet->payload[offset+extension_offset], 2);
-		  extension_offset += 2;
+		    memcpy(&extension_id, &packet->payload[offset+extension_offset], 2);
+		    extension_offset += 2;
 
-		  memcpy(&extension_len, &packet->payload[offset+extension_offset], 2);
-		  extension_offset += 2;
+		    memcpy(&extension_len, &packet->payload[offset+extension_offset], 2);
+		    extension_offset += 2;
 
-		  extension_id = ntohs(extension_id), extension_len = ntohs(extension_len);
+		    extension_id = ntohs(extension_id), extension_len = ntohs(extension_len);
 
-		  if(extension_id == 0) {
-		    u_int begin = 0,len;
-		    char *server_name = (char*)&packet->payload[offset+extension_offset];
+		    if(extension_id == 0) {
+		      u_int begin = 0,len;
+		      char *server_name = (char*)&packet->payload[offset+extension_offset];
 
-		    while(begin < extension_len) {
-		      if((!ndpi_isprint(server_name[begin]))
-			 || ndpi_ispunct(server_name[begin])
-			 || ndpi_isspace(server_name[begin]))
-			begin++;
-		      else
-			break;
-		    }
+		      while(begin < extension_len) {
+			if((!ndpi_isprint(server_name[begin]))
+			   || ndpi_ispunct(server_name[begin])
+			   || ndpi_isspace(server_name[begin]))
+			  begin++;
+			else
+			  break;
+		      }
 
-		    len = (u_int)ndpi_min(extension_len-begin, buffer_len-1);
-		    strncpy(buffer, &server_name[begin], len);
-		    buffer[len] = '\0';
-		    stripCertificateTrailer(buffer, buffer_len);
+		      len = (u_int)ndpi_min(extension_len-begin, buffer_len-1);
+		      strncpy(buffer, &server_name[begin], len);
+		      buffer[len] = '\0';
+		      stripCertificateTrailer(buffer, buffer_len);
 
-		    snprintf(flow->protos.ssl.client_certificate,
-			     sizeof(flow->protos.ssl.client_certificate), "%s", buffer);
+		      snprintf(flow->protos.ssl.client_certificate,
+			       sizeof(flow->protos.ssl.client_certificate), "%s", buffer);
 
-		    /* We're happy now */
-		    return(2 /* Client Certificate */);
-		  }
+		      /* We're happy now */
+		      return(2 /* Client Certificate */);
+		    }
 
-		  extension_offset += extension_len;
+		    extension_offset += extension_len;
+		  }
 		}
 	      }
 	    }