From: Drew Kluemke <[email protected]> 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 <[email protected]> Signed-off-by: Sriram Sriram <[email protected]> --- 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
