1. Patchew
  2. EDK2
  3. [edk2-devel] [edk2-platforms][PATCH v3 00/11] Ext4Pkg: Code correctness and security improvements
  4. View patch

[edk2-devel] [edk2-platforms][PATCH v3 07/11] Ext4Pkg: Check that source file is directory in Ext4OpenInternal

Savva Mitrofanov posted 11 patches 3 years ago
There is a newer version of this series
[edk2-devel] [edk2-platforms][PATCH v3 07/11] Ext4Pkg: Check that source file is directory in Ext4OpenInternal
Posted by Savva Mitrofanov 3 years ago 
This check already present in the while loop below, but absent for cases
when input file is nameless, so to handle assertion in Ext4ReadFile we
need to add it at the top of function

Cc: Marvin Häuser <mhaeuser@posteo.de>
Cc: Pedro Falcato <pedro.falcato@gmail.com>
Cc: Vitaly Cheptsov <vit9696@protonmail.com>
Fixes: d9ceedca6c8f ("Ext4Pkg: Add Ext4Dxe driver.")
Signed-off-by: Savva Mitrofanov <savvamtr@gmail.com>
---
 Features/Ext4Pkg/Ext4Dxe/File.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/Features/Ext4Pkg/Ext4Dxe/File.c b/Features/Ext4Pkg/Ext4Dxe/File.c
index 8dfe324255f4..9dde4a5d1a2d 100644
--- a/Features/Ext4Pkg/Ext4Dxe/File.c
+++ b/Features/Ext4Pkg/Ext4Dxe/File.c
@@ -207,6 +207,11 @@ Ext4OpenInternal (
   Level     = 0;

 

   DEBUG ((DEBUG_FS, "[ext4] Ext4OpenInternal %s\n", FileName));

+

+  if (!Ext4FileIsDir (Current)) {

+    return EFI_INVALID_PARAMETER;

+  }

+

   // If the path starts with a backslash, we treat the root directory as the base directory

   if (FileName[0] == L'\\') {

     FileName++;

@@ -219,6 +224,10 @@ Ext4OpenInternal (
       return EFI_ACCESS_DENIED;

     }

 

+    if (!Ext4FileIsDir (Current)) {

+      return EFI_INVALID_PARAMETER;

+    }

+

     // Discard leading path separators

     while (FileName[0] == L'\\') {

       FileName++;

@@ -242,10 +251,6 @@ Ext4OpenInternal (
 

     DEBUG ((DEBUG_FS, "[ext4] Opening %s\n", PathSegment));

 

-    if (!Ext4FileIsDir (Current)) {

-      return EFI_INVALID_PARAMETER;

-    }

-

     if (!Ext4IsLastPathSegment (FileName)) {

       if (!Ext4DirCanLookup (Current)) {

         return EFI_ACCESS_DENIED;

-- 
2.39.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#99143): https://edk2.groups.io/g/devel/message/99143
Mute This Topic: https://groups.io/mt/96562696/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [edk2-platforms][PATCH v3 07/11] Ext4Pkg: Check that source file is directory in Ext4OpenInternal
Posted by Pedro Falcato 3 years ago 
On Fri, Jan 27, 2023 at 9:29 AM Savva Mitrofanov <savvamtr@gmail.com> wrote:
>
> This check already present in the while loop below, but absent for cases
> when input file is nameless, so to handle assertion in Ext4ReadFile we
> need to add it at the top of function
>
> Cc: Marvin Häuser <mhaeuser@posteo.de>
> Cc: Pedro Falcato <pedro.falcato@gmail.com>
> Cc: Vitaly Cheptsov <vit9696@protonmail.com>
> Fixes: d9ceedca6c8f ("Ext4Pkg: Add Ext4Dxe driver.")
> Signed-off-by: Savva Mitrofanov <savvamtr@gmail.com>
> ---
>  Features/Ext4Pkg/Ext4Dxe/File.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/Features/Ext4Pkg/Ext4Dxe/File.c b/Features/Ext4Pkg/Ext4Dxe/File.c
> index 8dfe324255f4..9dde4a5d1a2d 100644
> --- a/Features/Ext4Pkg/Ext4Dxe/File.c
> +++ b/Features/Ext4Pkg/Ext4Dxe/File.c
> @@ -207,6 +207,11 @@ Ext4OpenInternal (
>    Level     = 0;
>
>    DEBUG ((DEBUG_FS, "[ext4] Ext4OpenInternal %s\n", FileName));
> +
> +  if (!Ext4FileIsDir (Current)) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
>    // If the path starts with a backslash, we treat the root directory as the base directory
>    if (FileName[0] == L'\\') {
>      FileName++;
> @@ -219,6 +224,10 @@ Ext4OpenInternal (
>        return EFI_ACCESS_DENIED;
>      }
>
> +    if (!Ext4FileIsDir (Current)) {
> +      return EFI_INVALID_PARAMETER;
> +    }
> +
>      // Discard leading path separators
>      while (FileName[0] == L'\\') {
>        FileName++;
> @@ -242,10 +251,6 @@ Ext4OpenInternal (
>
>      DEBUG ((DEBUG_FS, "[ext4] Opening %s\n", PathSegment));
>
> -    if (!Ext4FileIsDir (Current)) {
> -      return EFI_INVALID_PARAMETER;
> -    }
> -
>      if (!Ext4IsLastPathSegment (FileName)) {
>        if (!Ext4DirCanLookup (Current)) {
>          return EFI_ACCESS_DENIED;
> --
> 2.39.0
>

Reviewed-by: Pedro Falcato <pedro.falcato@gmail.com>

-- 
Pedro


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#99169): https://edk2.groups.io/g/devel/message/99169
Mute This Topic: https://groups.io/mt/96562696/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [edk2-platforms][PATCH v3 07/11] Ext4Pkg: Check that source file is directory in Ext4OpenInternal
Posted by Marvin Häuser 3 years ago 
Reviewed-by: Marvin Häuser <mhaeuser@posteo.de>

> On 27. Jan 2023, at 15:26, Pedro Falcato <pedro.falcato@gmail.com> wrote:
> 
> On Fri, Jan 27, 2023 at 9:29 AM Savva Mitrofanov <savvamtr@gmail.com> wrote:
>> 
>> This check already present in the while loop below, but absent for cases
>> when input file is nameless, so to handle assertion in Ext4ReadFile we
>> need to add it at the top of function
>> 
>> Cc: Marvin Häuser <mhaeuser@posteo.de>
>> Cc: Pedro Falcato <pedro.falcato@gmail.com>
>> Cc: Vitaly Cheptsov <vit9696@protonmail.com>
>> Fixes: d9ceedca6c8f ("Ext4Pkg: Add Ext4Dxe driver.")
>> Signed-off-by: Savva Mitrofanov <savvamtr@gmail.com>
>> ---
>> Features/Ext4Pkg/Ext4Dxe/File.c | 13 +++++++++----
>> 1 file changed, 9 insertions(+), 4 deletions(-)
>> 
>> diff --git a/Features/Ext4Pkg/Ext4Dxe/File.c b/Features/Ext4Pkg/Ext4Dxe/File.c
>> index 8dfe324255f4..9dde4a5d1a2d 100644
>> --- a/Features/Ext4Pkg/Ext4Dxe/File.c
>> +++ b/Features/Ext4Pkg/Ext4Dxe/File.c
>> @@ -207,6 +207,11 @@ Ext4OpenInternal (
>>   Level     = 0;
>> 
>>   DEBUG ((DEBUG_FS, "[ext4] Ext4OpenInternal %s\n", FileName));
>> +
>> +  if (!Ext4FileIsDir (Current)) {
>> +    return EFI_INVALID_PARAMETER;
>> +  }
>> +
>>   // If the path starts with a backslash, we treat the root directory as the base directory
>>   if (FileName[0] == L'\\') {
>>     FileName++;
>> @@ -219,6 +224,10 @@ Ext4OpenInternal (
>>       return EFI_ACCESS_DENIED;
>>     }
>> 
>> +    if (!Ext4FileIsDir (Current)) {
>> +      return EFI_INVALID_PARAMETER;
>> +    }
>> +
>>     // Discard leading path separators
>>     while (FileName[0] == L'\\') {
>>       FileName++;
>> @@ -242,10 +251,6 @@ Ext4OpenInternal (
>> 
>>     DEBUG ((DEBUG_FS, "[ext4] Opening %s\n", PathSegment));
>> 
>> -    if (!Ext4FileIsDir (Current)) {
>> -      return EFI_INVALID_PARAMETER;
>> -    }
>> -
>>     if (!Ext4IsLastPathSegment (FileName)) {
>>       if (!Ext4DirCanLookup (Current)) {
>>         return EFI_ACCESS_DENIED;
>> --
>> 2.39.0
>> 
> 
> Reviewed-by: Pedro Falcato <pedro.falcato@gmail.com>
> 
> -- 
> Pedro



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#99173): https://edk2.groups.io/g/devel/message/99173
Mute This Topic: https://groups.io/mt/96562696/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.