On 12. Jan 2023, at 00:59, Pedro Falcato <pedro.falcato@gmail.com> wrote:

Fix an out-of-bounds read inside CompareMem() when
checking for "." or ".." by explicitly bounding name_len
to [0, 2] beforehand.

Reported-by: Savva Mitrofanov <savvamtr@gmail.com>
Fixes: 45e37d8533ca8 ("Ext4Pkg: Hide "." and ".." entries from Read() callers.")
Cc: Marvin Häuser <mhaeuser@posteo.de>
Signed-off-by: Pedro Falcato <pedro.falcato@gmail.com>
---
Features/Ext4Pkg/Ext4Dxe/Directory.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/Features/Ext4Pkg/Ext4Dxe/Directory.c b/Features/Ext4Pkg/Ext4Dxe/Directory.c
index 4441e6d192b6..6ed664fc632f 100644
--- a/Features/Ext4Pkg/Ext4Dxe/Directory.c
+++ b/Features/Ext4Pkg/Ext4Dxe/Directory.c
@@ -491,12 +491,14 @@ Ext4ReadDir (
    // 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);
+    // We must bound name_len as > 0 and <= 2 to avoid any out-of-bounds accesses or bad detection of

+    // "." and "..".
+    IsDotOrDotDot = Entry.name_len > 0 && Entry.name_len <= 2 &&
+                    CompareMem (Entry.name, "..", Entry.name_len) == 0;

-    // When inode = 0, it's unused.
-    ShouldSkip = Entry.inode == 0 || IsDotOrDotDot;
+    // When inode = 0, it's unused. When name_len == 0, it's a nameless entry

+    // (which we should not expose to ReadDir).
+    ShouldSkip = Entry.inode == 0 || Entry.name_len == 0 || IsDotOrDotDot;

    if (ShouldSkip) {
      Offset += Entry.rec_len;
--
2.39.0