[PATCH net] ipv4: validate ip_forward_options() option fields against skb tail

[PATCH net] ipv4: validate ip_forward_options() option fields against skb tail

[Qi Tang]

ip_forward_options() re-reads the RR/SRR/TS option length byte
optptr[1] and pointer byte optptr[2] from the skb on the forwarding
path and uses them as indexes for 4-byte writes via
ip_rt_get_source() (and a memcmp walk in the SRR branch).

__ip_options_compile() validates those bytes at parse time but stores
only the option's offset into IPCB(skb)->opt.{rr,srr,ts}.  An nftables
FORWARD-chain payload mutation between parse and consume can rewrite
the bytes, driving the indexed writes out of bounds and overlapping
skb_shared_info.  With optptr[2] mutated the write can land in
skb_shared_info.frag_list; the next time the skb is dropped
kfree_skb_list_reason() walks the forged list and frees an
attacker-controlled pointer, an arbitrary-free primitive (R15 below
is the corrupted frag_list):

  BUG: unable to handle page fault for address: ffffed10195fd757
  Oops: 0000 [#1] SMP KASAN NOPTI
  RIP: 0010:kfree_skb_list_reason+0x167/0x5f0
  RAX: 1ffff110195fd757 RBX: dffffc0000000000
  R15: ffff8880cafebabe
  CR2: ffffed10195fd757
  Call Trace:
   skb_release_data+0x565/0x820
   sk_skb_reason_drop+0xc1/0x350
   ip_rcv_core+0x7a8/0xcd0
   ip_rcv+0x97/0x270
   __netif_receive_skb_one_core+0x161/0x1b0
   process_backlog+0x1c4/0x5b0
   net_rx_action+0x934/0xfa0

Bound optptr[2] within optptr[1] before the RR and TS writes, and
clamp the SRR walk to the bytes actually present in the skb.  Match
the existing error handling in this function: skip the malformed
option in place rather than returning, so the single ip_send_check()
at the end still recomputes the checksum for any option that was
updated earlier.

Cc: stable@vger.kernel.org
Reported-by: Qi Tang <tpluszz77@gmail.com>
Reported-by: Tong Liu <lyutoon@gmail.com>
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Qi Tang <tpluszz77@gmail.com>
---
 net/ipv4/ip_options.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/net/ipv4/ip_options.c b/net/ipv4/ip_options.c
index be8815ce3ac24..36a4e3cc39dd1 100644
--- a/net/ipv4/ip_options.c
+++ b/net/ipv4/ip_options.c
@@ -544,18 +544,26 @@ void ip_forward_options(struct sk_buff *skb)
 
 	if (opt->rr_needaddr) {
 		optptr = (unsigned char *)raw + opt->rr;
-		ip_rt_get_source(&optptr[optptr[2]-5], skb, rt);
-		opt->is_changed = 1;
+		if (optptr + optptr[1] <= skb_tail_pointer(skb) &&
+		    optptr[2] >= 5 && optptr[2] <= optptr[1] + 1) {
+			ip_rt_get_source(&optptr[optptr[2] - 5], skb, rt);
+			opt->is_changed = 1;
+		}
 	}
 	if (opt->srr_is_hit) {
 		int srrptr, srrspace;
 
 		optptr = raw + opt->srr;
 
-		for ( srrptr = optptr[2], srrspace = optptr[1];
-		     srrptr <= srrspace;
-		     srrptr += 4
-		     ) {
+		/* optptr[1] (option length) may have been rewritten after the
+		 * parse-time check; if it now runs past the skb the option is
+		 * malformed, so skip the source-route rewrite below.
+		 */
+		srrspace = optptr[1];
+		if (optptr + srrspace > skb_tail_pointer(skb))
+			srrspace = 0;
+
+		for (srrptr = optptr[2]; srrptr <= srrspace; srrptr += 4) {
 			if (srrptr + 3 > srrspace)
 				break;
 			if (memcmp(&opt->nexthop, &optptr[srrptr-1], 4) == 0)
@@ -572,8 +580,11 @@ void ip_forward_options(struct sk_buff *skb)
 		}
 		if (opt->ts_needaddr) {
 			optptr = raw + opt->ts;
-			ip_rt_get_source(&optptr[optptr[2]-9], skb, rt);
-			opt->is_changed = 1;
+			if (optptr + optptr[1] <= skb_tail_pointer(skb) &&
+			    optptr[2] >= 9 && optptr[2] <= optptr[1] + 5) {
+				ip_rt_get_source(&optptr[optptr[2] - 9], skb, rt);
+				opt->is_changed = 1;
+			}
 		}
 	}
 	if (opt->is_changed) {
-- 
2.47.3