Reviewed-by: Marvin Häuser <mhaeuser@posteo.de <mailto:mhaeuser@posteo.de>>
(Still two comments for next time)
> 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
"bad detection" = false positive?
> + // "." 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
Inconsistency between "=" and "==".
> + // (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 (#98513): https://edk2.groups.io/g/devel/message/98513
Mute This Topic: https://groups.io/mt/96212631/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-