Imporoved SMBv1 heuristic to avoid triggering risks for SMBv1 broadcast messages when used to browse (old) network devices

2 files changed, 34 insertions, 4 deletions

diff --git a/src/lib/ndpi_main.c b/src/lib/ndpi_main.c
index 1327c3975..aeae555a3 100644
--- a/src/lib/ndpi_main.c
+++ b/src/lib/ndpi_main.c
@@ -7716,7 +7716,7 @@ static void ndpi_reconcile_msteams_call_udp(struct ndpi_flow_struct *flow) {
 static void ndpi_reconcile_protocols(struct ndpi_detection_module_struct *ndpi_str,
 				     struct ndpi_flow_struct *flow,
 				     ndpi_protocol *ret) {
-  u_int i;
+  u_int i, skip_risk = 0;
 
   /* This function can NOT access &ndpi_str->packet since it is called also from ndpi_detection_giveup() */
 
@@ -7863,13 +7863,31 @@ static void ndpi_reconcile_protocols(struct ndpi_detection_module_struct *ndpi_s
     case NDPI_PROTOCOL_UNSAFE:
     case NDPI_PROTOCOL_POTENTIALLY_DANGEROUS:
     case NDPI_PROTOCOL_DANGEROUS:
-      ndpi_set_risk(flow, NDPI_UNSAFE_PROTOCOL, NULL);
+
+      if(flow->detected_protocol_stack[i] == NDPI_PROTOCOL_SMBV1) {
+	/*
+	  Same as for smb.c we need to avoid sending warnings for
+	  requests sent to a broadcast address that can be sent to
+	  query old devices. As we see no MAC addresses in nDPI
+	  it's not simple to detect this fact, so we will use some
+	  heuristic here.
+	*/
+
+	if(ndpi_str->packet.payload_packet_len > 86 /* SMB command */) {
+	  if(ndpi_str->packet.payload[86] == 0x25 /* SMB Trans */)
+	    skip_risk = 1;
+	}
+      }
+      
+      if(!skip_risk)
+	ndpi_set_risk(flow, NDPI_UNSAFE_PROTOCOL, NULL);
       break;
+      
     default:
       /* Nothing to do */
       break;
     }
-  }
+  } /* for */
 }
 
 /* ********************************************************************************* */
diff --git a/src/lib/protocols/smb.c b/src/lib/protocols/smb.c
index b0255cb63..3733b3747 100644
--- a/src/lib/protocols/smb.c
+++ b/src/lib/protocols/smb.c
@@ -52,7 +52,19 @@ static void ndpi_search_smb_tcp(struct ndpi_detection_module_struct *ndpi_struct
           if(packet->payload[8] != 0x72) /* Skip Negotiate request */ {
             NDPI_LOG_INFO(ndpi_struct, "found SMBv1\n");
             ndpi_set_detected_protocol(ndpi_struct, flow, NDPI_PROTOCOL_SMBV1, NDPI_PROTOCOL_NETBIOS, NDPI_CONFIDENCE_DPI);
-            ndpi_set_risk(flow, NDPI_SMB_INSECURE_VERSION, "Found SMBv1");
+
+	    /*
+	      Before we complain let's check if this is a broadacast message
+	      as for broadcast we can tolerate v1 as it can be used to
+	      discover old device versions.
+
+	      As nDPI has not MAC address visibility (checking for destination MAC
+	      FF:FF:FF:FF:FF:FF would have been easier) we need to implement
+	      some heuristic here.
+	    */
+
+	    if(packet->payload[8] != 0x25) /* Skip SMB command Trans */
+	      ndpi_set_risk(flow, NDPI_SMB_INSECURE_VERSION, "Found SMBv1");
           }
           return;
         } else if(memcmp(&packet->payload[4], smbv2, sizeof(smbv2)) == 0) {