From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f54.google.com (mail-wm1-f54.google.com [209.85.128.54]) by mx.groups.io with SMTP id smtpd.web09.11633.1629557303527170107 for ; Sat, 21 Aug 2021 07:48:23 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@gmail.com header.s=20161025 header.b=NkK6HfRq; spf=pass (domain: gmail.com, ip: 209.85.128.54, mailfrom: pedro.falcato@gmail.com) Received: by mail-wm1-f54.google.com with SMTP id f10so7639148wml.2 for ; Sat, 21 Aug 2021 07:48:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=i3PMpVig+hGkQwkyzIYD655lfT2tzZc7u4lg4I9KKG0=; b=NkK6HfRqXiVxTh09PlQfIMDsndeuSiDzikwLYGh5Rjt5uGx/0XySzQA17nO4uEJB9g HEgjK+XFV74aEaAbkA0JM3J9LJknh7DSwXZJcgqvAFIKR2ku6g6buG/nwUqo08HGPiyW 5x3EqVdZRgqZ1NiGZ0DxC609xSpw+H64NA9WUIxxPe/2l1Tl5Yheub4iR4fFWiZ3Hns+ 8Omi9RrO50pM7r8yH8jZdES3r1aCDgXWK1h2l60BuEB3Ke0PafEUcg6l7eLYYLikdnRt tGbC6rydLnJXw16IEktrJcJfLmcOmpUrd5frO3KkNLZXMGX+3GMEWhLOKE9/oac4fjAK b4oQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=i3PMpVig+hGkQwkyzIYD655lfT2tzZc7u4lg4I9KKG0=; b=fyJQbtifbwPSUoLlCgsNTpKF5KzgeFwVDadV5kOYMXt2mYrewpZr9IrzmIp09n8k0n 9G1arPKXXuDC9fGtkLPeKsDjpD/zNaKeUX+NYKOBjkl33ligMMtpRsDJY+N/JWY4SjMk DlTAW2V/FZ2soLP2nlmQjSApwjgvy1pfIEEBWdK6NsOXb7q1gVp2wuwt3vd7DT6+dthG vAsR6QxMAmOPsBxRn2jkfPq6vaiyRptp4PqGAvDVM1nyEXj1PJatKTdmsaIbUSvGMLXK l4bqHUMogAhihFw4jR7/F7MuVvw1AxVEzFTaHgdtiRkFJftRdaFEVky3KJ/xztTHkrKP 0C/w== X-Gm-Message-State: AOAM533FVcwYG76O329r8e62/19wMAebhzebXke6gfjViGNesHUjzHxj 4bSv4yo6MZfAn0F2iOTpTi4rqS7ofM029d6j X-Google-Smtp-Source: ABdhPJyx7L1J5udP3fOV4djgAKm6HQ1m00lFvpzjxhiEgm3T+Ia3COjZ/e50v9Mz/l+dZuaJWAGOTw== X-Received: by 2002:a7b:c756:: with SMTP id w22mr8646139wmk.169.1629557301950; Sat, 21 Aug 2021 07:48:21 -0700 (PDT) Return-Path: Received: from PC-PEDRO.lan (bl8-253-151.dsl.telepac.pt. [85.241.253.151]) by smtp.gmail.com with ESMTPSA id j17sm9118036wrt.69.2021.08.21.07.48.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 21 Aug 2021 07:48:21 -0700 (PDT) From: "Pedro Falcato" To: devel@edk2.groups.io Cc: Pedro Falcato , Leif Lindholm , Michael D Kinney , Bret Barkelew Subject: [edk2-platforms PATCH v2 5/5] Ext4Pkg: Sanity check more EXT4_DIR_ENTRY values. Date: Sat, 21 Aug 2021 15:47:10 +0100 Message-Id: <20210821144711.39546-6-pedro.falcato@gmail.com> X-Mailer: git-send-email 2.33.0 In-Reply-To: <20210821144711.39546-1-pedro.falcato@gmail.com> References: <20210821144711.39546-1-pedro.falcato@gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable This should close up some possible exploits using crafted filesystem images. Cc: Leif Lindholm Cc: Michael D Kinney Cc: Bret Barkelew Signed-off-by: Pedro Falcato --- Features/Ext4Pkg/Ext4Dxe/Directory.c | 90 ++++++++++++++++------------ 1 file changed, 51 insertions(+), 39 deletions(-) diff --git a/Features/Ext4Pkg/Ext4Dxe/Directory.c b/Features/Ext4Pkg/Ext4Dx= e/Directory.c index 7d1b2dcfe524..102c82f05da0 100644 --- a/Features/Ext4Pkg/Ext4Dxe/Directory.c +++ b/Features/Ext4Pkg/Ext4Dxe/Directory.c @@ -50,6 +50,37 @@ Ext4GetUcs2DirentName ( return Status;=0D }=0D =0D +/**=0D + Validates a directory entry.=0D +=0D + @param[in] Dirent Pointer to the directory entry.=0D +=0D + @retval TRUE Valid directory entry.=0D + FALSE Invalid directory entry.=0D +**/=0D +STATIC=0D +BOOLEAN=0D +Ext4ValidDirent (=0D + IN CONST EXT4_DIR_ENTRY *Dirent=0D + )=0D +{=0D + UINTN RequiredSize;=0D +=0D + RequiredSize =3D Dirent->name_len + EXT4_MIN_DIR_ENTRY_LEN;=0D +=0D + if (Dirent->rec_len < RequiredSize) {=0D + DEBUG ((DEBUG_ERROR, "[ext4] dirent size %lu too small (compared to %l= u)\n", Dirent->rec_len, RequiredSize));=0D + return FALSE;=0D + }=0D +=0D + // Dirent sizes need to be 4 byte aligned=0D + if (Dirent->rec_len % 4) {=0D + return FALSE;=0D + }=0D +=0D + return TRUE;=0D +}=0D +=0D /**=0D Retrieves a directory entry.=0D =0D @@ -75,11 +106,11 @@ Ext4RetrieveDirent ( UINT64 DirInoSize;=0D UINT32 BlockRemainder;=0D UINTN Length;=0D - CHAR8 *BufPtr;=0D EXT4_DIR_ENTRY *Entry;=0D UINTN RemainingBlock;=0D CHAR16 DirentUcs2Name[EXT4_NAME_MAX + 1];=0D UINTN ToCopy;=0D + UINTN BlockOffset;=0D =0D Status =3D EFI_NOT_FOUND;=0D Buf =3D AllocatePool (Partition->BlockSize);=0D @@ -109,14 +140,19 @@ Ext4RetrieveDirent ( return Status;=0D }=0D =0D - for (BufPtr =3D Buf; BufPtr < Buf + Partition->BlockSize; ) {=0D - Entry =3D (EXT4_DIR_ENTRY *)BufPtr;=0D - if (Entry->rec_len =3D=3D 0) {=0D + for (BlockOffset =3D 0; BlockOffset < Partition->BlockSize; ) {=0D + Entry =3D (EXT4_DIR_ENTRY *)(Buf + BlockOffset);=0D + RemainingBlock =3D Partition->BlockSize - BlockOffset;=0D + // Check if the minimum directory entry fits inside [BlockOffset, En= dOfBlock]=0D + if (RemainingBlock < EXT4_MIN_DIR_ENTRY_LEN) {=0D FreePool (Buf);=0D return EFI_VOLUME_CORRUPTED;=0D }=0D =0D - RemainingBlock =3D Partition->BlockSize - (BufPtr - Buf);=0D + if (!Ext4ValidDirent (Entry)) {=0D + FreePool (Buf);=0D + return EFI_VOLUME_CORRUPTED;=0D + }=0D =0D if (Entry->name_len > RemainingBlock || Entry->rec_len > RemainingBl= ock) {=0D // Corrupted filesystem=0D @@ -131,12 +167,13 @@ Ext4RetrieveDirent ( 2) Linux and a number of BSDs also have a filename limit of 255.=0D */=0D if (Entry->name_len > EXT4_NAME_MAX) {=0D + BlockOffset +=3D Entry->rec_len;=0D continue;=0D }=0D =0D // Unused entry=0D if (Entry->inode =3D=3D 0) {=0D - BufPtr +=3D Entry->rec_len;=0D + BlockOffset +=3D Entry->rec_len;=0D continue;=0D }=0D =0D @@ -150,7 +187,7 @@ Ext4RetrieveDirent ( if (EFI_ERROR (Status)) {=0D // If we error out, skip this entry=0D // I'm not sure if this is correct behaviour, but I don't think th= ere's a precedent here.=0D - BufPtr +=3D Entry->rec_len;=0D + BlockOffset +=3D Entry->rec_len;=0D continue;=0D }=0D =0D @@ -163,7 +200,7 @@ Ext4RetrieveDirent ( return EFI_SUCCESS;=0D }=0D =0D - BufPtr +=3D Entry->rec_len;=0D + BlockOffset +=3D Entry->rec_len;=0D }=0D =0D Off +=3D Partition->BlockSize;=0D @@ -379,37 +416,6 @@ Ext4OpenVolume ( return EFI_SUCCESS;=0D }=0D =0D -/**=0D - Validates a directory entry.=0D -=0D - @param[in] Dirent Pointer to the directory entry.=0D -=0D - @retval TRUE Valid directory entry.=0D - FALSE Invalid directory entry.=0D -**/=0D -STATIC=0D -BOOLEAN=0D -Ext4ValidDirent (=0D - IN CONST EXT4_DIR_ENTRY *Dirent=0D - )=0D -{=0D - UINTN RequiredSize;=0D -=0D - RequiredSize =3D Dirent->name_len + EXT4_MIN_DIR_ENTRY_LEN;=0D -=0D - if (Dirent->rec_len < RequiredSize) {=0D - DEBUG ((DEBUG_ERROR, "[ext4] dirent size %lu too small (compared to %l= u)\n", Dirent->rec_len, RequiredSize));=0D - return FALSE;=0D - }=0D -=0D - // Dirent sizes need to be 4 byte aligned=0D - if (Dirent->rec_len % 4) {=0D - return FALSE;=0D - }=0D -=0D - return TRUE;=0D -}=0D -=0D /**=0D Reads a directory entry.=0D =0D @@ -481,6 +487,12 @@ Ext4ReadDir ( goto Out;=0D }=0D =0D + // Check if the entire dir entry length fits in Len=0D + if (Len < EXT4_MIN_DIR_ENTRY_LEN + Entry.name_len) {=0D + Status =3D EFI_VOLUME_CORRUPTED;=0D + goto Out;=0D + }=0D +=0D // We don't care about passing . or .. entries to the caller of ReadDi= r(),=0D // since they're generally useless entries *and* may break things if t= oo=0D // many callers assume FAT32.=0D --=20 2.33.0