CVE-2020-15473: Out-of-bounds Read (CWE-125)
value: In nDPI through 3.2, the OpenVPN dissector is vulnerable to a heap-based buffer over-read in ndpi_search_openvpn in lib/protocols/openvpn.c.
CWE description: The product reads data past the end, or before the beginning, of the intended buffer.
CVSS v3 base score: 9.1
Published: 2020-07-01T11:15Z
Repository: https://github.com/ntop/nDPI
Language: C
Files changed: src/lib/protocols/openvpn.c
Commit date: 2020-05-15 21:10:37 +0200
Commit message: Fix for potential heap-buffer-overflow in ndpi_search_openvpn

Diff: diff --git a/src/lib/protocols/openvpn.c b/src/lib/protocols/openvpn.c
index 2753dd02..f0e3428c 100644
--- a/src/lib/protocols/openvpn.c
+++ b/src/lib/protocols/openvpn.c
@@ -70,33 +70,34 @@ void ndpi_search_openvpn(struct ndpi_detection_module_struct* ndpi_struct,
   const u_int8_t * session_remote;
   u_int8_t opcode;
   u_int8_t alen;
   int8_t hmac_size;
   int8_t failed = 0;
-
-  if(packet->payload_packet_len >= 40) {
+  /* No u_ */int16_t ovpn_payload_len = packet->payload_packet_len;
+  
+  if(ovpn_payload_len >= 40) {
     // skip openvpn TCP transport packet size
     if(packet->tcp != NULL)
-      ovpn_payload += 2;
+      ovpn_payload += 2, ovpn_payload_len -= 2;;
 
     opcode = ovpn_payload[0] & P_OPCODE_MASK;
 
     if(packet->udp) {
 #ifdef DEBUG
       printf("[packet_id: %u][opcode: %u][Packet ID: %d][%u <-> %u][len: %u]\n",
 	     flow->num_processed_pkts,
 	     opcode, check_pkid_and_detect_hmac_size(ovpn_payload),
-	     htons(packet->udp->source), htons(packet->udp->dest), packet->payload_packet_len);	   
+	     htons(packet->udp->source), htons(packet->udp->dest), ovpn_payload_len);	   
 #endif
       
       if(
 	 (flow->num_processed_pkts == 1)
 	 && (
-	     ((packet->payload_packet_len == 112)
+	     ((ovpn_payload_len == 112)
 	      && ((opcode == 168) || (opcode == 192))
 	      )
-	     || ((packet->payload_packet_len == 80)
+	     || ((ovpn_payload_len == 80)
 		 && ((opcode == 184) || (opcode == 88) || (opcode == 160) || (opcode == 168) || (opcode == 200)))
 	     )) {
 	NDPI_LOG_INFO(ndpi_struct,"found openvpn\n");
 	ndpi_set_detected_protocol(ndpi_struct, flow, NDPI_PROTOCOL_OPENVPN, NDPI_PROTOCOL_UNKNOWN);
 	return;
@@ -117,26 +118,34 @@ void ndpi_search_openvpn(struct ndpi_detection_module_struct* ndpi_struct,
             (opcode == P_CONTROL_HARD_RESET_SERVER_V1 || opcode == P_CONTROL_HARD_RESET_SERVER_V2)) {
 
       hmac_size = check_pkid_and_detect_hmac_size(ovpn_payload);
 
       if(hmac_size > 0) {
-        alen = ovpn_payload[P_PACKET_ID_ARRAY_LEN_OFFSET(hmac_size)];
+	u_int16_t offset = P_PACKET_ID_ARRAY_LEN_OFFSET(hmac_size);
+	  
+        alen = ovpn_payload[offset];
+	
         if (alen > 0) {
-	  session_remote = ovpn_payload + P_PACKET_ID_ARRAY_LEN_OFFSET(hmac_size) + 1 + alen * 4;
-
-          if(memcmp(flow->ovpn_session_id, session_remote, 8) == 0) {
-	    NDPI_LOG_INFO(ndpi_struct,"found openvpn\n");
-	    ndpi_set_detected_protocol(ndpi_struct, flow, NDPI_PROTOCOL_OPENVPN, NDPI_PROTOCOL_UNKNOWN);
-	    return;
-	  } else {
-            NDPI_LOG_DBG2(ndpi_struct,
-		   "key mismatch: %02x%02x%02x%02x%02x%02x%02x%02x\n",
-		   session_remote[0], session_remote[1], session_remote[2], session_remote[3],
-		   session_remote[4], session_remote[5], session_remote[6], session_remote[7]);
-            failed = 1;
-          }
-        } else
+	  offset += 1 + alen * 4;
+
+	  if((offset+8) <= ovpn_payload_len) {
+	    session_remote = &ovpn_payload[offset];
+	    
+	    if(memcmp(flow->ovpn_session_id, session_remote, 8) == 0) {
+	      NDPI_LOG_INFO(ndpi_struct,"found openvpn\n");
+	      ndpi_set_detected_protocol(ndpi_struct, flow, NDPI_PROTOCOL_OPENVPN, NDPI_PROTOCOL_UNKNOWN);
+	      return;
+	    } else {
+	      NDPI_LOG_DBG2(ndpi_struct,
+			    "key mismatch: %02x%02x%02x%02x%02x%02x%02x%02x\n",
+			    session_remote[0], session_remote[1], session_remote[2], session_remote[3],
+			    session_remote[4], session_remote[5], session_remote[6], session_remote[7]);
+	      failed = 1;
+	    }
+	  } else
+	    failed = 1;
+	} else
           failed = 1;
       } else
         failed = 1;
     } else
       failed = 1;

Vulnerable code: 
  if(packet->payload_packet_len >= 40) {
      ovpn_payload += 2;
	     htons(packet->udp->source), htons(packet->udp->dest), packet->payload_packet_len);	   
	     ((packet->payload_packet_len == 112)
	     || ((packet->payload_packet_len == 80)
        alen = ovpn_payload[P_PACKET_ID_ARRAY_LEN_OFFSET(hmac_size)];
	  session_remote = ovpn_payload + P_PACKET_ID_ARRAY_LEN_OFFSET(hmac_size) + 1 + alen * 4;

          if(memcmp(flow->ovpn_session_id, session_remote, 8) == 0) {
	    NDPI_LOG_INFO(ndpi_struct,"found openvpn\n");
	    ndpi_set_detected_protocol(ndpi_struct, flow, NDPI_PROTOCOL_OPENVPN, NDPI_PROTOCOL_UNKNOWN);
	    return;
	  } else {
            NDPI_LOG_DBG2(ndpi_struct,
		   "key mismatch: %02x%02x%02x%02x%02x%02x%02x%02x\n",
		   session_remote[0], session_remote[1], session_remote[2], session_remote[3],
		   session_remote[4], session_remote[5], session_remote[6], session_remote[7]);
            failed = 1;
          }
        } else
Fixed code:   /* No u_ */int16_t ovpn_payload_len = packet->payload_packet_len;
  
  if(ovpn_payload_len >= 40) {
      ovpn_payload += 2, ovpn_payload_len -= 2;;
	     htons(packet->udp->source), htons(packet->udp->dest), ovpn_payload_len);	   
	     ((ovpn_payload_len == 112)
	     || ((ovpn_payload_len == 80)
	u_int16_t offset = P_PACKET_ID_ARRAY_LEN_OFFSET(hmac_size);
	  
        alen = ovpn_payload[offset];
	
	  offset += 1 + alen * 4;

	  if((offset+8) <= ovpn_payload_len) {
	    session_remote = &ovpn_payload[offset];
	    
	    if(memcmp(flow->ovpn_session_id, session_remote, 8) == 0) {
	      NDPI_LOG_INFO(ndpi_struct,"found openvpn\n");
	      ndpi_set_detected_protocol(ndpi_struct, flow, NDPI_PROTOCOL_OPENVPN, NDPI_PROTOCOL_UNKNOWN);
	      return;
	    } else {
	      NDPI_LOG_DBG2(ndpi_struct,
			    "key mismatch: %02x%02x%02x%02x%02x%02x%02x%02x\n",
			    session_remote[0], session_remote[1], session_remote[2], session_remote[3],
			    session_remote[4], session_remote[5], session_remote[6], session_remote[7]);
	      failed = 1;
	    }
	  } else
	    failed = 1;
	} else