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 <savva...@gmail.com>
Fixes: 45e37d8533ca8 ("Ext4Pkg: Hide "." and ".." entries from Read() callers.")
Cc: Marvin Häuser <mhaeu...@posteo.de>
Signed-off-by: Pedro Falcato <pedro.falc...@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
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#98325): https://edk2.groups.io/g/devel/message/98325
Mute This Topic: https://groups.io/mt/96212631/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
