From patchwork Thu May 26 12:45:16 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Fabio Estevam X-Patchwork-Id: 1635850 X-Patchwork-Delegate: rfried.dev@gmail.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20210112 header.b=WtfOzhEo; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=lists.denx.de (client-ip=85.214.62.61; helo=phobos.denx.de; envelope-from=u-boot-bounces@lists.denx.de; receiver=) Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4L872R0qwgz9sFk for ; Thu, 26 May 2022 22:45:34 +1000 (AEST) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 60ADF83D7F; Thu, 26 May 2022 14:45:29 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="WtfOzhEo"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id CE7C983340; Thu, 26 May 2022 14:45:27 +0200 (CEST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on phobos.denx.de X-Spam-Level: X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,SPF_HELO_NONE, SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.2 Received: from mail-oa1-x2e.google.com (mail-oa1-x2e.google.com [IPv6:2001:4860:4864:20::2e]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 2E6CA83D7F for ; Thu, 26 May 2022 14:45:23 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=festevam@gmail.com Received: by mail-oa1-x2e.google.com with SMTP id 586e51a60fabf-f2c8c0d5bdso2043739fac.4 for ; Thu, 26 May 2022 05:45:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=F483rH4vTXVgWjTQ5vdqY1DKzUlWtUOBHMqB81LZnuM=; b=WtfOzhEoTYz4MPt6BthRhTZLDOc+Zl7bAY40SOw7H4hBZnNaXxGvUawZ9P6ypOTnfm rq6NkL1WloWspYRRxeldcJ8VQpTWxtgwRdf/MxURXlcbZ3a4R7K4oe93fMJ4OXNzays7 m/H4XrHc3qgJKEHEqBRwT2ie/1UlTeymDHVg/dwZ2xNMSvtjzP0NwJ8QuZDSupGvnms+ gEMT131lDD9E/Hl0cI1E/YE4t/pAwNLjNkNZBKN+RmQtxeKkAVtTeGC9eZobveyVeV8v 7J0/YajTPkhmNP6wNzUNgg1i0D6exUe74hJzFPrw7Fzr1tNBPwinMdFrqY280tSzAouv LmRA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=F483rH4vTXVgWjTQ5vdqY1DKzUlWtUOBHMqB81LZnuM=; b=2xTkL1kb4jH8pt6ZSxTl1B+WoXm71/enRR5BIcHashOQe8k7TL7fgW6v+sUUcOPNuo LwANEryL7dAapWt5il7LJU4zhJbWRwLnLLWDxJJTsPJ7Th/qy7jeLav7qVEXgRk02bEC EuWw8wRFDJaSZT5RilkZjiJFJDcai+mQ05oE4EDvmYpt5RlKiR7hdTexG4a7c855VYiN c0OU8xxd2H0N3+0ECMIISBFYiQaIKQFZeBYPSerlAjdkqc2B+3UHifhxFTa1DD/pVfpq NTJPdmVXnZNsvTFgGuCgm3mJJ3h5XMIegaA+W1OFpxcBuZinfZ+Eqgy7wMtLAK3g+YyQ 8yKQ== X-Gm-Message-State: AOAM531SyadfLnhkS7vw1U8pyG3eXqnXJPCBwNa7Pkg725mU7l2lXyA+ /S1GT52fhdqwiddVIkNy6m8= X-Google-Smtp-Source: ABdhPJzxI40XLi3lfJnqk1MdTXn9dvdpyDg7O6MdQWhsyVJdcRB2QqqB9SegjE5P8rrU8hk7FplsJg== X-Received: by 2002:a05:6870:471f:b0:f1:92a5:117f with SMTP id b31-20020a056870471f00b000f192a5117fmr1129520oaq.162.1653569121773; Thu, 26 May 2022 05:45:21 -0700 (PDT) Received: from localhost.localdomain ([2804:14c:485:4b69:ec34:4110:ea9d:c402]) by smtp.gmail.com with ESMTPSA id b18-20020a4a9fd2000000b0035eb4e5a6b8sm601236oom.14.2022.05.26.05.45.18 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 26 May 2022 05:45:21 -0700 (PDT) From: Fabio Estevam To: rfried.dev@gmail.com Cc: joe.hershberger@ni.com, u-boot@lists.denx.de, nicolas.bidron@nccgroup.com, michael@amarulasolutions.com, mbrugger@suse.com, trini@konsulko.com, Fabio Estevam Subject: [PATCH] net: Check for the minimum IP fragmented datagram size Date: Thu, 26 May 2022 09:45:16 -0300 Message-Id: <20220526124516.1229280-1-festevam@gmail.com> X-Mailer: git-send-email 2.25.1 MIME-Version: 1.0 X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.5 at phobos.denx.de X-Virus-Status: Clean From: Fabio Estevam Nicolas Bidron and Nicolas Guigo reported the two bugs below: " ----------BUG 1---------- In compiled versions of U-Boot that define CONFIG_IP_DEFRAG, a value of `ip->ip_len` (IP packet header's Total Length) higher than `IP_HDR_SIZE` and strictly lower than `IP_HDR_SIZE+8` will lead to a value for `len` comprised between `0` and `7`. This will ultimately result in a truncated division by `8` resulting value of `0` forcing the hole metadata and fragment to point to the same location. The subsequent memcopy will overwrite the hole metadata with the fragment data. Through a second fragment, this can be exploited to write to an arbitrary offset controlled by that overwritten hole metadata value. This bug is only exploitable locally as it requires crafting two packets the first of which would most likely be dropped through routing due to its unexpectedly low Total Length. However, this bug can potentially be exploited to root linux based embedded devices locally. ```C static struct ip_udp_hdr *__net_defragment(struct ip_udp_hdr *ip, int *lenp) { static uchar pkt_buff[IP_PKTSIZE] __aligned(PKTALIGN); static u16 first_hole, total_len; struct hole *payload, *thisfrag, *h, *newh; struct ip_udp_hdr *localip = (struct ip_udp_hdr *)pkt_buff; uchar *indata = (uchar *)ip; int offset8, start, len, done = 0; u16 ip_off = ntohs(ip->ip_off); /* payload starts after IP header, this fragment is in there */ payload = (struct hole *)(pkt_buff + IP_HDR_SIZE); offset8 = (ip_off & IP_OFFS); thisfrag = payload + offset8; start = offset8 * 8; len = ntohs(ip->ip_len) - IP_HDR_SIZE; ``` The last line of the previous excerpt from `u-boot/net/net.c` shows how the attacker can control the value of `len` to be strictly lower than `8` by issuing a packet with `ip_len` between `21` and `27` (`IP_HDR_SIZE` has a value of `20`). Also note that `offset8` here is `0` which leads to `thisfrag = payload`. ```C } else if (h >= thisfrag) { /* overlaps with initial part of the hole: move this hole */ newh = thisfrag + (len / 8); *newh = *h; h = newh; if (h->next_hole) payload[h->next_hole].prev_hole = (h - payload); if (h->prev_hole) payload[h->prev_hole].next_hole = (h - payload); else first_hole = (h - payload); } else { ``` Lower down the same function, execution reaches the above code path. Here, `len / 8` evaluates to `0` leading to `newh = thisfrag`. Also note that `first_hole` here is `0` since `h` and `payload` point to the same location. ```C /* finally copy this fragment and possibly return whole packet */ memcpy((uchar *)thisfrag, indata + IP_HDR_SIZE, len); ``` Finally, in the above excerpt the `memcpy` overwrites the hole metadata since `thisfrag` and `h` both point to the same location. The hole metadata is effectively overwritten with arbitrary data from the fragmented IP packet data. If `len` was crafted to be `6`, `last_byte`, `next_hole`, and `prev_hole` of the `first_hole` can be controlled by the attacker. Finally the arbitrary offset write occurs through a second fragment that only needs to be crafted to write data in the hole pointed to by the previously controlled hole metadata (`next_hole`) from the first packet. ### Recommendation Handle cases where `len` is strictly lower than 8 by preventing the overwrite of the hole metadata during the memcpy of the fragment. This could be achieved by either: * Moving the location where the hole metadata is stored when `len` is lower than `8`. * Or outright rejecting fragmented IP datagram with a Total Length (`ip_len`) lower than 28 bytes which is the minimum valid fragmented IP datagram size (as defined as the minimum fragment of 8 octets in the IP Specification Document: [RFC791](https://datatracker.ietf.org/doc/html/rfc791) page 25). ----------BUG 2---------- In compiled versions of U-Boot that define CONFIG_IP_DEFRAG, a value of `ip->ip_len` (IP packet header's Total Length) lower than `IP_HDR_SIZE` will lead to a negative value for `len` which will ultimately result in a buffer overflow during the subsequent `memcpy` that uses `len` as it's `count` parameter. This bug is only exploitable on local ethernet as it requires crafting an invalid packet to include an unexpected `ip_len` value in the IP UDP header that's lower than the minimum accepted Total Length of a packet (21 as defined in the IP Specification Document: [RFC791](https://datatracker.ietf.org/doc/html/rfc791)). Such packet would in all likelihood be dropped while being routed to its final destination through most routing equipment and as such requires the attacker to be in a local position in order to be exploited. ```C static struct ip_udp_hdr *__net_defragment(struct ip_udp_hdr *ip, int *lenp) { static uchar pkt_buff[IP_PKTSIZE] __aligned(PKTALIGN); static u16 first_hole, total_len; struct hole *payload, *thisfrag, *h, *newh; struct ip_udp_hdr *localip = (struct ip_udp_hdr *)pkt_buff; uchar *indata = (uchar *)ip; int offset8, start, len, done = 0; u16 ip_off = ntohs(ip->ip_off); /* payload starts after IP header, this fragment is in there */ payload = (struct hole *)(pkt_buff + IP_HDR_SIZE); offset8 = (ip_off & IP_OFFS); thisfrag = payload + offset8; start = offset8 * 8; len = ntohs(ip->ip_len) - IP_HDR_SIZE; ``` The last line of the previous excerpt from `u-boot/net/net.c` shows where the underflow to a negative `len` value occurs if `ip_len` is set to a value strictly lower than 20 (`IP_HDR_SIZE` being 20). Also note that in the above excerpt the `pkt_buff` buffer has a size of `CONFIG_NET_MAXDEFRAG` which defaults to 16 KB but can range from 1KB to 64 KB depending on configurations. ```C /* finally copy this fragment and possibly return whole packet */ memcpy((uchar *)thisfrag, indata + IP_HDR_SIZE, len); ``` In the above excerpt the `memcpy` overflows the destination by attempting to make a copy of nearly 4 gigabytes in a buffer that's designed to hold `CONFIG_NET_MAXDEFRAG` bytes at most which leads to a DoS. ### Recommendation Stop processing of the packet if `ip_len` is lower than 21 (as defined by the minimum length of a data carrying datagram in the IP Specification Document: [RFC791](https://datatracker.ietf.org/doc/html/rfc791) page 34)." Add a check for ip_len lesser than 28 and stop processing the packet in this case. Such a check covers the two reported bugs. Reported-by: Nicolas Bidron Signed-off-by: Fabio Estevam --- include/net.h | 2 ++ net/net.c | 3 +++ 2 files changed, 5 insertions(+) diff --git a/include/net.h b/include/net.h index 675bf4171b17..8933b976fa07 100644 --- a/include/net.h +++ b/include/net.h @@ -391,6 +391,8 @@ struct ip_hdr { #define IP_HDR_SIZE (sizeof(struct ip_hdr)) +#define IP_MIN_FRAG_DATAGRAM_SIZE 28 + /* * Internet Protocol (IP) + UDP header. */ diff --git a/net/net.c b/net/net.c index 034a5d6e67c0..81905f631592 100644 --- a/net/net.c +++ b/net/net.c @@ -907,6 +907,9 @@ static struct ip_udp_hdr *__net_defragment(struct ip_udp_hdr *ip, int *lenp) int offset8, start, len, done = 0; u16 ip_off = ntohs(ip->ip_off); + if (ip->ip_len < IP_MIN_FRAG_DATAGRAM_SIZE) + return NULL; + /* payload starts after IP header, this fragment is in there */ payload = (struct hole *)(pkt_buff + IP_HDR_SIZE); offset8 = (ip_off & IP_OFFS);