net: lwip: tftp: fix find_option()

Find_option() is used to retrieve the block size value in an option
acknowledgment in response to a request containing a block size option
according to RFC2348.

The format of an OACK response is described in RFC2347 as

+-------+---~~---+---+---~~---+---+---~~---+---+---~~---+---+
|  opc  |  opt1  | 0 | value1 | 0 |  optN  | 0 | valueN | 0 |
+-------+---~~---+---+---~~---+---+---~~---+---+---~~---+---+

The current implementation of find_option() only works if

* blksize is the first option
* lwip_strnstr() ignores the length parameter,
  i.e. is implemented via strstr()

The OACK messages starts with  0x00 0x06. If 'blksize' is the first option,
strstr() reports a match when the first parameter points to 0x06. Adding
the string length of 'blksize' plus 2 to the location of the 0x06 byte
points to the value.

Find_option() would report a match for option 'blksize' if the response
contained an option called 'foo_blksize_bar'. In this case find_option()
would return 'bar' as the value string.

If 'blksize' were the second option, find_option() would return a pointer
to the second character of the value string.

Furthermore find_option() does not detect if the value string is NUL
terminated. This may lead to a buffer overrun.

Provide an implementation that correctly steps from option to option.

Fixes: 27d7ccda94fa ("net: lwip: tftp: add support of blksize option to client")
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org>
Tested-by: Jerome Forissier <jerome.forissier@linaro.org> (qemu_arm64_lwip)
Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>

diff --git a/lib/lwip/lwip/src/apps/tftp/tftp.c b/lib/lwip/lwip/src/apps/tftp/tftp.c
index 56aeabc4d7372c4b4339b7482e8d08a121d607db..63b1e0e0e207f43a1068ed306725509c1267cbfc 100644 (file)
--- a/lib/lwip/lwip/src/apps/tftp/tftp.c
+++ b/lib/lwip/lwip/src/apps/tftp/tftp.c
@@ -264,19 +264,55 @@ static u16_t payload_size(void)
   return TFTP_DEFAULT_BLOCK_SIZE;
 }
 
+/**
+ * find_option() - check if OACK message contains option
+ *
+ * @p:         message buffer
+ * @option:    option key
+ * Return:     option value
+ */
 static const char *
 find_option(struct pbuf *p, const char *option)
 {
-  int i;
-  u16_t optlen = strlen(option);
-  const char *b = p->payload;
-
-  for (i = 0; i + optlen + 1 < p->len; i++) {
-    if (lwip_strnstr(b + i, option, optlen))
-      return b + i + optlen + 2;
-  }
+       const char *pos = p->payload;
+       int rem = p->len;
+
+       /*
+        * According to RFC 2347 the OACK packet has the following format:
+        *
+        * +-------+---~~---+---+---~~---+---+---~~---+---+---~~---+---+
+        * |  opc  |  opt1  | 0 | value1 | 0 |  optN  | 0 | valueN | 0 |
+        * +-------+---~~---+---+---~~---+---+---~~---+---+---~~---+---+
+        */
+
+       /* Skip opc */
+       pos += 2;
+       rem -= 2;
+       if (rem <= 0)
+               return NULL;
+
+       for (;;) {
+               int len;
+               int diff;
+
+               len = strnlen(pos, rem) + 1;
+               if (rem < len)
+                       break;
+               diff = strcmp(pos, option);
+               /* Skip option */
+               pos += len;
+               rem -= len;
+               len = strnlen(pos, rem) + 1;
+               if (rem < len)
+                       break;
+               if (!diff)
+                       return pos;
+               /* Skip value */
+               pos += len;
+               rem -= len;
+       }
 
-  return NULL;
+       return NULL;
 }
 
 static void