From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 60207C43334 for ; Sat, 4 Jun 2022 18:07:34 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 7E55F83E43; Sat, 4 Jun 2022 20:07:31 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=reject dis=none) header.from=mu-ori.me 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=mu-ori.me header.i=@mu-ori.me header.b="arKPLTEf"; dkim=pass (2048-bit key) header.d=mu-ori.me header.i=@mu-ori.me header.b="arKPLTEf"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 3053283E60; Sat, 4 Jun 2022 20:07:29 +0200 (CEST) Received: from mail.mu-ori.me (mail.mu-ori.me [185.189.151.126]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 2B68183E38 for ; Sat, 4 Jun 2022 20:07:26 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=reject dis=none) header.from=mu-ori.me Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=gerbert@mu-ori.me Received: by mail.mu-ori.me (Postfix) id 691DC60018; Sat, 4 Jun 2022 18:07:25 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mu-ori.me; s=mail; t=1654366045; bh=a3NeAcNPLpqyyBDxsD4ZgfyVRdc8eAiAUD9uDt+syLw=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=arKPLTEfpjwANtgWslG5zEBpmOC7qMDLxRIH3t5r9uBF0J4LpHj1pFfDjpeK2hz1+ AaAd4Xm8jh1FBNn+ks1Ic39Y4RCbOWVoKgzkkcFG+dE2l8+qyHw0XwoNy8r7Wl91Ka 1aEsgRwLP4QlSU63w4kz6SWytaEjrDSBbwQDjRDebjGiRg/vRSKd1Musd13HI4Qjc2 ga5dlxuzDX61Y/eNY+EBLrS8TNXkwLroQR9tfglgzIxU2FPBaR+whKRVBLBX5aseQJ 8tLD8wYyASgsdx2GNNsDQcEukotiokH9DPuG6FOvyOlBvwNtwYoxut4zKikynAw0TH HvdkIfExQK9bA== Received: from webm.mu-ori.me (localhost [127.0.0.1]) by mail.mu-ori.me (Postfix) with ESMTPSA id 2EF605FEA9; Sat, 4 Jun 2022 18:07:25 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mu-ori.me; s=mail; t=1654366045; bh=a3NeAcNPLpqyyBDxsD4ZgfyVRdc8eAiAUD9uDt+syLw=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=arKPLTEfpjwANtgWslG5zEBpmOC7qMDLxRIH3t5r9uBF0J4LpHj1pFfDjpeK2hz1+ AaAd4Xm8jh1FBNn+ks1Ic39Y4RCbOWVoKgzkkcFG+dE2l8+qyHw0XwoNy8r7Wl91Ka 1aEsgRwLP4QlSU63w4kz6SWytaEjrDSBbwQDjRDebjGiRg/vRSKd1Musd13HI4Qjc2 ga5dlxuzDX61Y/eNY+EBLrS8TNXkwLroQR9tfglgzIxU2FPBaR+whKRVBLBX5aseQJ 8tLD8wYyASgsdx2GNNsDQcEukotiokH9DPuG6FOvyOlBvwNtwYoxut4zKikynAw0TH HvdkIfExQK9bA== MIME-Version: 1.0 Date: Sat, 04 Jun 2022 21:07:25 +0300 From: gerbert To: Heinrich Schuchardt Cc: u-boot@lists.denx.de Subject: Re: [PATCH 1/1] CVE-2022-30767: unbounded memcpy with a failed length check In-Reply-To: References: Message-ID: <2a4db65a2417c88c484244dc2bc0cc0c@mu-ori.me> X-Sender: gerbert@mu-ori.me User-Agent: Roundcube Webmail/1.3.16 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit 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 Heinrich Schuchardt писал 2022-06-04 20:44: > On 6/2/22 20:32, gerbert wrote: >> This patch tries to fix a CVE-2019-14196 fix >> >>   In if-condition, where NFSV2_FLAG is checked, memcpy call is >> performed >> to transfer a reply data of NFS_FHSIZE size. Since the data field in >> struct rpc_t structure has the size of (1024 / 4) + 26 = 282, while >> NFS_FHSIZE is only 32, it won't lead to out-of-bounds write >> (considering >> the size of data array won't change in the future). >> >>   What concerns if-condition for NFSV3_FLAG, since filefh3_length is >> signed integer, it may carry negative values which may lead to memcpy >> failure, so in this case we need to introduce not only boundary check >> (filefh3_length > NFS3_FHSIZE), which exists, but also make sure that >> filefh3_length is not negative. >> >> Signed-off-by: gerbert >> --- >>  net/nfs.c | 6 +++--- >>  1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/net/nfs.c b/net/nfs.c >> index 9152ab742e..5186130ea9 100644 >> --- a/net/nfs.c >> +++ b/net/nfs.c >> @@ -566,13 +566,13 @@ static int nfs_lookup_reply(uchar *pkt, unsigned >> len) >>      } >> >>      if (supported_nfs_versions & NFSV2_FLAG) { >> -        if (((uchar *)&(rpc_pkt.u.reply.data[0]) - (uchar >> *)(&rpc_pkt) >> + NFS_FHSIZE) > len) >> -            return -NFS_RPC_DROP; >>          memcpy(filefh, rpc_pkt.u.reply.data + 1, NFS_FHSIZE); >>      } else {  /* NFSV3_FLAG */ >>          filefh3_length = ntohl(rpc_pkt.u.reply.data[1]); >> +        if (filefh3_length < 0) > > This is the definition: > > static unsigned int filefh3_length > The value cannot be negative. That's right. It's unsigned now. Unfortunately I was looking into the version I was using in the project, which is, 2021.01. A bit outdated and has this field as signed integer. That's why I was thinking that this should work. Meanwhile, I then think that this part can be removed: >> -        if (((uchar *)&(rpc_pkt.u.reply.data[0]) - (uchar >> *)(&rpc_pkt) >> + NFS_FHSIZE) > len) as >>          memcpy(filefh, rpc_pkt.u.reply.data + 1, NFS_FHSIZE); won't leave the boundary of 'rpc_pkt.u.reply.data + 1' due to the size of NFS_FHSIZE. > > Cf. > bdbf7a05e26f3c5 ("net: nfs: Fix CVE-2022-30767 (old CVE-2019-14196)") > > >> +            return -NFS_RPC_DROP; >>          if (filefh3_length > NFS3_FHSIZE) >> -            filefh3_length  = NFS3_FHSIZE; >> +            filefh3_length = NFS3_FHSIZE; > > This seems to be an unrelated change. Thought about a little cosmetics here. > > Best regards > > Heinrich > >>          memcpy(filefh, rpc_pkt.u.reply.data + 2, filefh3_length); >>      } >> Kind regards, Gerbert