From afc2b641eb9cf5035b5147e78030bafe0b40dd87 Mon Sep 17 00:00:00 2001
From: Ivan Nardi <12729895+IvanNardi@users.noreply.github.com>
Date: Mon, 15 Nov 2021 16:20:57 +0100
Subject: Fix writes to `flow->protos` union fields (#1354)

We can write to `flow->protos` only after a proper classification.

This issue has been found in Kerberos, DHCP, HTTP, STUN, IMO, FTP,
SMTP, IMAP and POP code.
There are two kinds of fixes:
 * write to `flow->protos` only if a final protocol has been detected
 * move protocol state out of `flow->protos`
The hard part is to find, for each protocol, the right tradeoff between
memory usage and code complexity.

Handle Kerberos like DNS: if we find a request, we set the protocol
and an extra callback to further parsing the reply.

For all the other protocols, move the state out of `flow->protos`. This
is an issue only for the FTP/MAIL stuff.

Add DHCP Class Identification value to the output of ndpiReader and to
the Jason serialization.

Extend code coverage of fuzz tests.

Close #1343
Close #1342
---
 src/lib/protocols/ftp_control.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

(limited to 'src/lib/protocols/ftp_control.c')

diff --git a/src/lib/protocols/ftp_control.c b/src/lib/protocols/ftp_control.c
index 7b6544bb4..2b6f1396f 100644
--- a/src/lib/protocols/ftp_control.c
+++ b/src/lib/protocols/ftp_control.c
@@ -50,23 +50,23 @@ static int ndpi_ftp_control_check_request(struct ndpi_detection_module_struct *n
 #endif
 
   if(ndpi_match_strprefix(payload, payload_len, "USER")) {
-    ndpi_user_pwd_payload_copy((u_int8_t*)flow->protos.ftp_imap_pop_smtp.username,
-			       sizeof(flow->protos.ftp_imap_pop_smtp.username), 5,
+    ndpi_user_pwd_payload_copy((u_int8_t*)flow->ftp_imap_pop_smtp.username,
+			       sizeof(flow->ftp_imap_pop_smtp.username), 5,
 			       payload, payload_len);
     ndpi_set_risk(ndpi_struct, flow, NDPI_CLEAR_TEXT_CREDENTIALS);
     return 1;
   }
 
   if(ndpi_match_strprefix(payload, payload_len, "PASS")) {
-    ndpi_user_pwd_payload_copy((u_int8_t*)flow->protos.ftp_imap_pop_smtp.password,
-			       sizeof(flow->protos.ftp_imap_pop_smtp.password), 5,
+    ndpi_user_pwd_payload_copy((u_int8_t*)flow->ftp_imap_pop_smtp.password,
+			       sizeof(flow->ftp_imap_pop_smtp.password), 5,
 			       payload, payload_len);
     return 1;
   }
 
   if(ndpi_match_strprefix(payload, payload_len, "AUTH") ||
      ndpi_match_strprefix(payload, payload_len, "auth")) {
-    flow->protos.ftp_imap_pop_smtp.auth_found = 1;
+    flow->ftp_imap_pop_smtp.auth_found = 1;
     return 1;
   }
   /* ***************************************************** */
@@ -562,14 +562,14 @@ static int ndpi_ftp_control_check_response(struct ndpi_flow_struct *flow,
   case '2':
   case '3':
   case '6':
-    if(flow->protos.ftp_imap_pop_smtp.auth_found == 1)
-      flow->protos.ftp_imap_pop_smtp.auth_tls = 1;
+    if(flow->ftp_imap_pop_smtp.auth_found == 1)
+      flow->ftp_imap_pop_smtp.auth_tls = 1;
     return(1);
     break;
 
   case '4':
   case '5':
-    flow->protos.ftp_imap_pop_smtp.auth_failed = 1;
+    flow->ftp_imap_pop_smtp.auth_failed = 1;
     return(1);
     break;
   }
@@ -632,11 +632,11 @@ static void ndpi_check_ftp_control(struct ndpi_detection_module_struct *ndpi_str
 
 #ifdef FTP_DEBUG
 	printf("%s() [user: %s][pwd: %s]\n", __FUNCTION__,
-	       flow->protos.ftp_imap_pop_smtp.username, flow->protos.ftp_imap_pop_smtp.password);
+	       flow->ftp_imap_pop_smtp.username, flow->ftp_imap_pop_smtp.password);
 #endif
 
-	if(flow->protos.ftp_imap_pop_smtp.password[0] == '\0' &&
-	   flow->protos.ftp_imap_pop_smtp.auth_tls == 0) /* TODO: any values on dissecting TLS handshake? */
+	if(flow->ftp_imap_pop_smtp.password[0] == '\0' &&
+	   flow->ftp_imap_pop_smtp.auth_tls == 0) /* TODO: any values on dissecting TLS handshake? */
 	  flow->ftp_control_stage = 0;
 	else
 	  ndpi_int_ftp_control_add_connection(ndpi_struct, flow);
-- 
cgit v1.2.3