tftp: rework request checking to account for options

Read and write requests were not handling options.
The existing code inspecting request messages assumed that the string
before the end of the payload was the mode and that the filename
length depended on the mode length.
However the first two strings are the filename and mode which can be
followed by any number of option strings.

Rework the checking of the filename and mode to just search the first
two strings which should always exist. Any options after that are
ignored.

Absence of the filename or mode is now excludes the TFTP protocol.
Absence of the filename no longer is considered malformed packet
because it can match other protocols falsely.

1 files changed, 48 insertions, 22 deletions

diff --git a/src/lib/protocols/tftp.c b/src/lib/protocols/tftp.c
index dfdd60517..135d1bea2 100644
--- a/src/lib/protocols/tftp.c
+++ b/src/lib/protocols/tftp.c
@@ -65,43 +65,69 @@ static void ndpi_search_tftp(struct ndpi_detection_module_struct *ndpi_struct,
         }
 
         {
-          char const * const possible_modes[] = { "netascii", "octet", "mail" };
-          uint8_t mode_found = 0, mode_idx;
+          uint8_t mode_found = 0;
           size_t mode_len;
-
-          for(mode_idx = 0; mode_idx < NDPI_ARRAY_LENGTH(possible_modes); ++mode_idx)
+          int i;
+          size_t filename_len = 0;
+          const char *string;
+          size_t len = 0;
+          bool first = true;
+
+          /* Skip 2 byte opcode. */
+          for (i = 2; i < packet->payload_packet_len; i++)
           {
-            mode_len = strlen(possible_modes[mode_idx]);
-
-            if (packet->payload_packet_len < mode_len + 1 /* mode is a nul terminated string */)
+            /* Search through the payload until we find a NULL terminated string. */
+            if (packet->payload[i] != '\0')
             {
+              len++;
               continue;
             }
-            if (strncasecmp((char const *)&packet->payload[packet->payload_packet_len - 1 - mode_len],
-                            possible_modes[mode_idx], mode_len) == 0)
+            string = (const char *)&packet->payload[i - len];
+
+            /* Filename should be immediately after opcode followed by the mode. */
+            if (first)
             {
-              mode_found = 1;
-              break;
+              filename_len = len;
+              len = 0;
+              first = false;
+              continue;
             }
+
+            char const * const possible_modes[] = { "netascii", "octet", "mail" };
+            uint8_t mode_idx;
+
+            /* Check the string in the payload against the possible TFTP modes. */
+            for(mode_idx = 0; mode_idx < NDPI_ARRAY_LENGTH(possible_modes); ++mode_idx)
+              {
+                mode_len = strlen(possible_modes[mode_idx]);
+                /* Both are now null terminated */
+                if (len != mode_len)
+                {
+                  continue;
+                }
+                if (strncasecmp(string, possible_modes[mode_idx], mode_len) == 0)
+                {
+                  mode_found = 1;
+                  break;
+                }
+              }
+
+            /* Second string searched must've been the mode, break out before any following options. */
+            break;
           }
 
-          if (mode_found == 0)
+          /* Exclude the flow as TFPT if there was no filename and mode in the first two strings. */
+          if (filename_len == 0 || ndpi_is_printable_buffer(&packet->payload[2], filename_len) == 0 ||
+              mode_found == 0)
           {
             NDPI_EXCLUDE_PROTO(ndpi_struct, flow);
             return;
           }
 
           /* Dissect RRQ/WWQ filename. */
-          size_t filename_len = packet->payload_packet_len - 2 /* Opcode */ - mode_len - 1 /* NUL */;
-
-          if (filename_len == 0 || packet->payload[2] == '\0' || ndpi_is_printable_buffer(&packet->payload[2], filename_len - 1) == 0)
-          {
-            ndpi_set_risk(ndpi_struct, flow, NDPI_MALFORMED_PACKET, "Invalid TFTP RR/WR header: Source/Destination file missing");
-          } else {
-            filename_len = ndpi_min(filename_len, sizeof(flow->protos.tftp.filename) - 1);
-            memcpy(flow->protos.tftp.filename, &packet->payload[2], filename_len);
-            flow->protos.tftp.filename[filename_len] = '\0';
-          }
+          filename_len = ndpi_min(filename_len, sizeof(flow->protos.tftp.filename) - 1);
+          memcpy(flow->protos.tftp.filename, &packet->payload[2], filename_len);
+          flow->protos.tftp.filename[filename_len] = '\0';
 
           /* We have seen enough and do not need any more TFTP packets. */
           NDPI_LOG_INFO(ndpi_struct, "found tftp (RRQ/WWQ)\n");