public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Savva Mitrofanov" <savvamtr@gmail.com>
To: devel@edk2.groups.io
Cc: "Marvin Häuser" <mhaeuser@posteo.de>,
	"Pedro Falcato" <pedro.falcato@gmail.com>,
	"Vitaly Cheptsov" <vit9696@protonmail.com>
Subject: [edk2-platforms][PATCH v1 03/12] Ext4Pkg: Fix global buffer overflow in Ext4ReadDir
Date: Fri,  9 Dec 2022 22:10:55 +0600	[thread overview]
Message-ID: <20221209161104.70220-4-savvamtr@gmail.com> (raw)
In-Reply-To: <20221209161104.70220-1-savvamtr@gmail.com>

Directory entry structure can contain name_len bigger than size of "."
or "..", that's why CompareMem in such cases leads to global buffer
overflow. So there are two problems. The first is that statement doesn't
check cases when name_len != 0 but > 2 and the second is that we passing
big Length to CompareMem routine.
The correct way here is to check that name_len <= 2 and check for
null-terminator presence

Cc: Marvin Häuser <mhaeuser@posteo.de>
Cc: Pedro Falcato <pedro.falcato@gmail.com>
Cc: Vitaly Cheptsov <vit9696@protonmail.com>
Signed-off-by: Savva Mitrofanov <savvamtr@gmail.com>
---
 Features/Ext4Pkg/Ext4Dxe/Directory.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/Features/Ext4Pkg/Ext4Dxe/Directory.c b/Features/Ext4Pkg/Ext4Dxe/Directory.c
index 8b8fce568e43..ffc0e8043076 100644
--- a/Features/Ext4Pkg/Ext4Dxe/Directory.c
+++ b/Features/Ext4Pkg/Ext4Dxe/Directory.c
@@ -491,11 +491,9 @@ Ext4ReadDir (
 
     // Entry.name_len may be 0 if it's a nameless entry, like an unused entry
     // or a checksum at the end of the directory block.
-    // memcmp (and CompareMem) return 0 when the passed length is 0.
-
-    IsDotOrDotDot = Entry.name_len != 0 &&
-                    (CompareMem (Entry.name, ".", Entry.name_len) == 0 ||
-                     CompareMem (Entry.name, "..", Entry.name_len) == 0);
+    IsDotOrDotDot = Entry.name_len <= 2 &&
+                    ((Entry.name[0] == '.') &&
+                     (Entry.name[1] == '.' || Entry.name[1] == '\0'));
 
     // When inode = 0, it's unused.
     ShouldSkip = Entry.inode == 0 || IsDotOrDotDot;
-- 
2.38.1


  parent reply	other threads:[~2022-12-09 16:11 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-09 16:10 [edk2-platforms][PATCH v1 00/12] Ext4Pkg: Code correctness and security improvements Savva Mitrofanov
2022-12-09 16:10 ` [edk2-platforms][PATCH v1 01/12] Ext4Pkg: Fix memory leak in Ext4RetrieveDirent Savva Mitrofanov
2022-12-09 16:10 ` [edk2-platforms][PATCH v1 02/12] Ext4Pkg: Move EXT4_NAME_MAX definition to Ext4Disk.h Savva Mitrofanov
2022-12-09 16:10 ` Savva Mitrofanov [this message]
2022-12-09 16:10 ` [edk2-platforms][PATCH v1 04/12] Ext4Pkg: Fix incorrect checksum metadata feature check Savva Mitrofanov
2022-12-09 16:10 ` [edk2-platforms][PATCH v1 05/12] Ext4Pkg: Fix division by zero by adding check for s_inodes_per_group Savva Mitrofanov
2022-12-09 16:10 ` [edk2-platforms][PATCH v1 06/12] Ext4Pkg: Add comparison between Position and FileSize in Ext4SetPosition Savva Mitrofanov
2022-12-09 22:12   ` Pedro Falcato
2022-12-12 11:44     ` Savva Mitrofanov
2022-12-09 16:10 ` [edk2-platforms][PATCH v1 07/12] Ext4Pkg: Add inode number validity check Savva Mitrofanov
2022-12-09 16:11 ` [edk2-platforms][PATCH v1 08/12] Ext4Pkg: Fix shift out of bounds in Ext4OpenSuperblock Savva Mitrofanov
2022-12-09 16:11 ` [edk2-platforms][PATCH v1 09/12] Ext4Pkg: Correct integer overflow check on multiplication in DiskUtil Savva Mitrofanov
2022-12-09 22:16   ` Pedro Falcato
2022-12-09 16:11 ` [edk2-platforms][PATCH v1 10/12] Ext4Pkg: Check that source file is directory in Ext4OpenInternal Savva Mitrofanov
2022-12-09 16:11 ` [edk2-platforms][PATCH v1 11/12] Ext4Pkg: Check VolumeName allocation correctness in Ext4GetVolumeName Savva Mitrofanov
2022-12-09 16:11 ` [edk2-platforms][PATCH v1 12/12] Ext4Pkg: Add missing exit Status in Ext4OpenDirent Savva Mitrofanov
2022-12-09 22:28 ` [edk2-platforms][PATCH v1 00/12] Ext4Pkg: Code correctness and security improvements Pedro Falcato
2022-12-12 14:40   ` Savva Mitrofanov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221209161104.70220-4-savvamtr@gmail.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox