On 12. Dec 2022, at 15:46, Savva Mitrofanov wrote:
>
> 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
> Cc: Pedro Falcato
> Cc: Vitaly Cheptsov
> Fixes: e55f0527dde48a5f139c1b8f35acc4e6b59dd794
> Signed-off-by: Savva Mitrofanov
> ---
> 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'));
This is definitely borked, names do not need to be 0-terminated. So this may
cause OOB if Entry.name_len == 1 and Entry.name[0] == '.' and also may yield a
false negative.
>
> // When inode = 0, it's unused.
> ShouldSkip = Entry.inode == 0 || IsDotOrDotDot;
> --
> 2.38.1
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#97584): https://edk2.groups.io/g/devel/message/97584
Mute This Topic: https://groups.io/mt/95622331/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
