[PATCH v2 2/3] net: tftp: verify the OACK packet is NUL terminated

[Sriram Sriram <sriramsriram@linux.microsoft.com>]

From: Drew Kluemke <ankluemk@microsoft.com>

The OACK handler matches option names with strcasecmp() and parses the
values with dectoul(). Both scan until a NUL byte, so a malformed or
truncated OACK that contains no NUL within the received length makes
them read past the end of the packet buffer.

RFC 2347 formats an OACK as a sequence of NUL terminated netascii
strings, so the last byte of a well-formed packet is always a NUL.
Check for it once on entry and drop the packet if it is missing, which
keeps every strcasecmp() and dectoul() below in bounds without having to
bound each one individually.

Signed-off-by: Drew Kluemke <ankluemk@microsoft.com>
Signed-off-by: Sriram Sriram <sriramsriram@linux.microsoft.com>
---
Changes in v2:
 - Replaced the four bounded strncasecmp() comparisons with a single
   check that the packet ends in a NUL, per Tom's review. RFC 2347
   guarantees the trailing NUL, so one test covers every strcasecmp()
   and, unlike v1, the dectoul() calls as well.
 - Smaller: +16 bytes of .text on am335x_evm_defconfig, against +24 for
   v1 (net/tftp.o 3222 unpatched, 3246 in v1, 3238 here).
 - Retitled from "net: tftp: use bounded string compare for OACK option
   parsing" to match the new approach.

 net/tftp.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/net/tftp.c b/net/tftp.c
index beb9d08f5a2..73b31b8357d 100644
--- a/net/tftp.c
+++ b/net/tftp.c
@@ -528,6 +528,13 @@ static void tftp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
 #endif
 
 	case TFTP_OACK:
+		/*
+		 * RFC 2347 makes every option name and value a NUL terminated
+		 * string, so a well-formed OACK always ends in a NUL. Checking
+		 * that once keeps the option parsing below in bounds.
+		 */
+		if (!len || pkt[len - 1] != '\0')
+			return;
 		debug("Got OACK: ");
 		for (i = 0; i < len; i++) {
 			if (pkt[i] == '\0')
-- 
2.49.0