Skip site navigation (1)Skip section navigation (2) 
Date:      Mon, 23 Dec 2019 10:06:33 +0000 (UTC)
From:      "Andrey V. Elsukov" <ae@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-11@freebsd.org
Subject:   svn commit: r356036 - stable/11/sys/netpfil/ipfw
Message-ID:  <201912231006.xBNA6XJd057401@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: ae
Date: Mon Dec 23 10:06:32 2019
New Revision: 356036
URL: https://svnweb.freebsd.org/changeset/base/356036

Log:
  MFC r355712:
    Make TCP options parsing stricter.
  
    Rework tcpopts_parse() to be more strict. Use const pointer. Add length
    checks for specific TCP options. The main purpose of the change is
    avoiding of possible out of mbuf's data access.
  
    Reported by:	Maxime Villard

Modified:
  stable/11/sys/netpfil/ipfw/ip_fw2.c
Directory Properties:
  stable/11/   (props changed)

Modified: stable/11/sys/netpfil/ipfw/ip_fw2.c
==============================================================================
--- stable/11/sys/netpfil/ipfw/ip_fw2.c	Mon Dec 23 10:02:55 2019	(r356035)
+++ stable/11/sys/netpfil/ipfw/ip_fw2.c	Mon Dec 23 10:06:32 2019	(r356036)
@@ -328,22 +328,27 @@ ipopts_match(struct ip *ip, ipfw_insn *cmd)
 	return (flags_match(cmd, bits));
 }
 
+/*
+ * Parse TCP options. The logic copied from tcp_dooptions().
+ */
 static int
-tcpopts_parse(struct tcphdr *tcp, uint16_t *mss)
+tcpopts_parse(const struct tcphdr *tcp, uint16_t *mss)
 {
-	u_char *cp = (u_char *)(tcp + 1);
+	const u_char *cp = (const u_char *)(tcp + 1);
 	int optlen, bits = 0;
-	int x = (tcp->th_off << 2) - sizeof(struct tcphdr);
+	int cnt = (tcp->th_off << 2) - sizeof(struct tcphdr);
 
-	for (; x > 0; x -= optlen, cp += optlen) {
+	for (; cnt > 0; cnt -= optlen, cp += optlen) {
 		int opt = cp[0];
 		if (opt == TCPOPT_EOL)
 			break;
 		if (opt == TCPOPT_NOP)
 			optlen = 1;
 		else {
+			if (cnt < 2)
+				break;
 			optlen = cp[1];
-			if (optlen <= 0)
+			if (optlen < 2 || optlen > cnt)
 				break;
 		}
 
@@ -352,22 +357,31 @@ tcpopts_parse(struct tcphdr *tcp, uint16_t *mss)
 			break;
 
 		case TCPOPT_MAXSEG:
+			if (optlen != TCPOLEN_MAXSEG)
+				break;
 			bits |= IP_FW_TCPOPT_MSS;
 			if (mss != NULL)
 				*mss = be16dec(cp + 2);
 			break;
 
 		case TCPOPT_WINDOW:
-			bits |= IP_FW_TCPOPT_WINDOW;
+			if (optlen == TCPOLEN_WINDOW)
+				bits |= IP_FW_TCPOPT_WINDOW;
 			break;
 
 		case TCPOPT_SACK_PERMITTED:
+			if (optlen == TCPOLEN_SACK_PERMITTED)
+				bits |= IP_FW_TCPOPT_SACK;
+			break;
+
 		case TCPOPT_SACK:
-			bits |= IP_FW_TCPOPT_SACK;
+			if (optlen > 2 && (optlen - 2) % TCPOLEN_SACK == 0)
+				bits |= IP_FW_TCPOPT_SACK;
 			break;
 
 		case TCPOPT_TIMESTAMP:
-			bits |= IP_FW_TCPOPT_TS;
+			if (optlen == TCPOLEN_TIMESTAMP)
+				bits |= IP_FW_TCPOPT_TS;
 			break;
 		}
 	}

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201912231006.xBNA6XJd057401>