From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f50.google.com (mail-wr1-f50.google.com [209.85.221.50]) by mx.groups.io with SMTP id smtpd.web10.2380.1683592927647173930 for ; Mon, 08 May 2023 17:42:07 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="signature has expired" header.i=@gmail.com header.s=20221208 header.b=T8ZdtrOO; spf=pass (domain: gmail.com, ip: 209.85.221.50, mailfrom: pedro.falcato@gmail.com) Received: by mail-wr1-f50.google.com with SMTP id ffacd0b85a97d-304935cc79bso4844496f8f.2 for ; Mon, 08 May 2023 17:42:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1683592926; x=1686184926; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=83ix5URvtbwwmV1mdMjSka7GaDPWCXsTZ0B/y4hNQ7A=; b=T8ZdtrOORU0fMi7TpUT88XoqtoGOMBLF+kAs2IePHTnI/2A25t2IB7hXiyeG5m6ZJt lbXdaaySMRWwfem88PFtrV88RMFw3yuccIPXcN7uZb45eBxRTD1pS+T/KSB3aS3XRRMs HmPKtjvNcl8fsxlqF1vySniGruX3Zruxam61bA19IyP15DoHZmU+D8YR2aGU/j/v5nEk 9ehTeT1r/KILipP9Toewe+cmTseNTqGqPdMSPKS3uhsmOgAI48CnHQNXOSUzR94zsYKG NsC6Lp92Xi5OZRZX0sr3wfd10RLNF96pIinpk1IvWtr8c2A00hzFpfBxREP69LthYZQG AbIw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1683592926; x=1686184926; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=83ix5URvtbwwmV1mdMjSka7GaDPWCXsTZ0B/y4hNQ7A=; b=PJhIHJePTU6y4Uz9kXurj7zs+x/bfADOFvNBH8GyMyjtSpHF/TKW2tzQU8jL8NAPSS MyYuomBoVi4G4l+C2QRrIYcVFJf4ewgNbER6EIF3efRHgalZVHcjyc90zHMGgw2KhiQH ADleWl97WL/XMATr+77sI681tE8w9q/xsEF4axLR9jXUJ0S2S1bSaQkYbO0UvfgA9uVj 5SilhpPcetRjQ6DOkxKo0AZWjQBtdVG7Pd5wdBMEmc8jYbXdXJs3pYSjJ5HKyG00j4bG Kz8a5aAiAcgkTAA7En1RDdJOSNuxzz9u4G8L5gb50cmHtWT9RmoS+6AQ7KVVrw1APojE 5UiQ== X-Gm-Message-State: AC+VfDylyT625bIyU9TRinCasJBlEdqsTX7S8sFwak7ThFubqW7O1AAB rkhM3Np7YpK0ga5G+mJur+aj/39Ze/H7Iovg X-Google-Smtp-Source: ACHHUZ4YInRUiR2r9Gsg/EtGZmN7emh/UT1i9AzZUqIeu4W3sWJsMV6/rSMqP4mJEO24YnreElajbw== X-Received: by 2002:adf:f9ca:0:b0:2ee:f1f0:14bb with SMTP id w10-20020adff9ca000000b002eef1f014bbmr7221802wrr.49.1683592925585; Mon, 08 May 2023 17:42:05 -0700 (PDT) Return-Path: Received: from PC-PEDRO-ARCH.lan ([2001:8a0:7280:5801:9441:3dce:686c:bfc7]) by smtp.gmail.com with ESMTPSA id r15-20020a5d694f000000b00306415ac69asm12612219wrw.15.2023.05.08.17.42.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 08 May 2023 17:42:05 -0700 (PDT) From: "Pedro Falcato" To: devel@edk2.groups.io Cc: Pedro Falcato , =?UTF-8?q?Marvin=20H=C3=A4user?= , Savva Mitrofanov Subject: [PATCH 1/2] Ext4Pkg: Improve extent node validation on the number of entries Date: Tue, 9 May 2023 01:42:01 +0100 Message-Id: <20230509004202.243859-1-pedro.falcato@gmail.com> X-Mailer: git-send-email 2.40.1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Improve the extent tree node validation by validating the number of entries the node advertises against the theoretical max (derived from the size of on-disk structs and the block size (or i_data, if inline extents). Previously, we did not validate the number of entries. This could be exploited for out-of-bounds reads and crashes. Cc: Marvin Häuser Fixes: d9ceedca6c8f ("Ext4Pkg: Add Ext4Dxe driver.") Reported-by: Savva Mitrofanov Signed-off-by: Pedro Falcato --- Features/Ext4Pkg/Ext4Dxe/Extents.c | 47 +++++++++++++++++++----------- 1 file changed, 30 insertions(+), 17 deletions(-) diff --git a/Features/Ext4Pkg/Ext4Dxe/Extents.c b/Features/Ext4Pkg/Ext4Dxe/Extents.c index 9e4364e50d99..66d085938549 100644 --- a/Features/Ext4Pkg/Ext4Dxe/Extents.c +++ b/Features/Ext4Pkg/Ext4Dxe/Extents.c @@ -1,7 +1,7 @@ /** @file Extent related routines - Copyright (c) 2021 - 2022 Pedro Falcato All rights reserved. + Copyright (c) 2021 - 2023 Pedro Falcato All rights reserved. SPDX-License-Identifier: BSD-2-Clause-Patent **/ @@ -80,13 +80,15 @@ Ext4GetInoExtentHeader ( /** Checks if an extent header is valid. @param[in] Header Pointer to the EXT4_EXTENT_HEADER structure. + @param[in] MaxEntries Maximum number of entries possible for this tree node. @return TRUE if valid, FALSE if not. **/ STATIC BOOLEAN Ext4ExtentHeaderValid ( - IN CONST EXT4_EXTENT_HEADER *Header + IN CONST EXT4_EXTENT_HEADER *Header, + IN UINT16 MaxEntries ) { if (Header->eh_depth > EXT4_EXTENT_TREE_MAX_DEPTH) { @@ -99,6 +101,18 @@ Ext4ExtentHeaderValid ( return FALSE; } + if ((Header->eh_max > MaxEntries) || (Header->eh_entries > MaxEntries)) { + DEBUG (( + DEBUG_ERROR, + "[ext4] Invalid extent header entries (extent header: %u max," + " %u entries, theoretical max: %u) (larger than permitted)\n", + Header->eh_max, + Header->eh_entries, + MaxEntries + )); + return FALSE; + } + if (Header->eh_max < Header->eh_entries) { DEBUG (( DEBUG_ERROR, @@ -212,6 +226,9 @@ Ext4ExtentIdxLeafBlock ( return LShiftU64 (Index->ei_leaf_hi, 32) | Index->ei_leaf_lo; } +// Results of sizeof(i_data) / sizeof(extent) - 1 = 4 +#define EXT4_NR_INLINE_EXTENTS 4 + /** Retrieves an extent from an EXT4 inode. @param[in] Partition Pointer to the opened EXT4 partition. @@ -237,7 +254,7 @@ Ext4GetExtent ( EXT4_EXTENT_HEADER *ExtHeader; EXT4_EXTENT_INDEX *Index; EFI_STATUS Status; - EXT4_BLOCK_NR BlockNumber; + UINT32 MaxExtentsPerNode; Inode = File->Inode; Ext = NULL; @@ -275,12 +292,17 @@ Ext4GetExtent ( ExtHeader = Ext4GetInoExtentHeader (Inode); - if (!Ext4ExtentHeaderValid (ExtHeader)) { + if (!Ext4ExtentHeaderValid (ExtHeader, EXT4_NR_INLINE_EXTENTS)) { return EFI_VOLUME_CORRUPTED; } CurrentDepth = ExtHeader->eh_depth; + // A single node fits into a single block, so we can only have (BlockSize / sizeof(EXT4_EXTENT)) - 1 + // extents in a single node. Note the -1, because both leaf and internal node headers are 12 bytes, + // and so are individual entries. + MaxExtentsPerNode = (Partition->BlockSize / sizeof (EXT4_EXTENT)) - 1; + while (ExtHeader->eh_depth != 0) { CurrentDepth--; // While depth != 0, we're traversing the tree itself and not any leaves @@ -289,17 +311,7 @@ Ext4GetExtent ( // Therefore, we can use binary search, and it's actually the standard for doing so // (see FreeBSD). - Index = Ext4BinsearchExtentIndex (ExtHeader, LogicalBlock); - BlockNumber = Ext4ExtentIdxLeafBlock (Index); - - // Check that block isn't file hole - if (BlockNumber == EXT4_BLOCK_FILE_HOLE) { - if (Buffer != NULL) { - FreePool (Buffer); - } - - return EFI_VOLUME_CORRUPTED; - } + Index = Ext4BinsearchExtentIndex (ExtHeader, LogicalBlock); if (Buffer == NULL) { Buffer = AllocatePool (Partition->BlockSize); @@ -309,7 +321,8 @@ Ext4GetExtent ( } // Read the leaf block onto the previously-allocated buffer. - Status = Ext4ReadBlocks (Partition, Buffer, 1, BlockNumber); + + Status = Ext4ReadBlocks (Partition, Buffer, 1, Ext4ExtentIdxLeafBlock (Index)); if (EFI_ERROR (Status)) { FreePool (Buffer); return Status; @@ -317,7 +330,7 @@ Ext4GetExtent ( ExtHeader = Buffer; - if (!Ext4ExtentHeaderValid (ExtHeader)) { + if (!Ext4ExtentHeaderValid (ExtHeader, MaxExtentsPerNode)) { FreePool (Buffer); return EFI_VOLUME_CORRUPTED; } -- 2.40.1