From: "Savva Mitrofanov" <savvamtr@gmail.com>
To: devel@edk2.groups.io
Cc: "Marvin Häuser" <mhaeuser@posteo.de>,
"Pedro Falcato" <pedro.falcato@gmail.com>,
"Vitaly Cheptsov" <vit9696@protonmail.com>
Subject: [edk2-platforms][PATCH v1 06/12] Ext4Pkg: Add comparison between Position and FileSize in Ext4SetPosition
Date: Fri, 9 Dec 2022 22:10:58 +0600 [thread overview]
Message-ID: <20221209161104.70220-7-savvamtr@gmail.com> (raw)
In-Reply-To: <20221209161104.70220-1-savvamtr@gmail.com>
Missing such comparison leads to infinite loop states, for example code
which trying to read entire file can easily get out of bound of
file size by passing position value which exceeds file size without this
check. So we need to add there missing comparison between the desired
position to be set and file size
Cc: Marvin Häuser <mhaeuser@posteo.de>
Cc: Pedro Falcato <pedro.falcato@gmail.com>
Cc: Vitaly Cheptsov <vit9696@protonmail.com>
Signed-off-by: Savva Mitrofanov <savvamtr@gmail.com>
---
Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h | 19 +++++++++---------
Features/Ext4Pkg/Ext4Dxe/File.c | 21 +++++++++++++-------
2 files changed, 23 insertions(+), 17 deletions(-)
diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
index dde4f4cb0e06..1dcb644e3b35 100644
--- a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
+++ b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
@@ -31,7 +31,7 @@
#include "Ext4Disk.h"
-#define SYMLOOP_MAX 8
+#define SYMLOOP_MAX 8
//
// We need to specify path length limit for security purposes, to prevent possible
// overflows and dead-loop conditions. Originally this limit is absent in FS design,
@@ -715,16 +715,15 @@ Ext4GetPosition (
/**
Sets a file's current position.
- @param[in] This A pointer to the EFI_FILE_PROTOCOL instance that
-is the file handle to set the requested position on.
- @param[in] Position The byte position from the start of the file to
-set.
+ @param[in] This A pointer to the EFI_FILE_PROTOCOL instance that is the
+ file handle to set the requested position on.
+ @param[in] Position The byte position from the start of the file to set.
- @retval EFI_SUCCESS The position was set.
- @retval EFI_UNSUPPORTED The seek request for nonzero is not valid on open
- directories.
- @retval EFI_DEVICE_ERROR An attempt was made to set the position of a deleted
-file.
+ @retval EFI_SUCCESS The position was set.
+ @retval EFI_INVALID_PARAMETER The seek request for non-zero position is not valid on open
+ directories.
+ @retval EFI_UNSUPPORTED The seek request for position is exceeds FileSize.
+ @retval EFI_DEVICE_ERROR An attempt was made to set the position of a deleted file.
**/
EFI_STATUS
diff --git a/Features/Ext4Pkg/Ext4Dxe/File.c b/Features/Ext4Pkg/Ext4Dxe/File.c
index 04198a53bfc0..b4ed78847258 100644
--- a/Features/Ext4Pkg/Ext4Dxe/File.c
+++ b/Features/Ext4Pkg/Ext4Dxe/File.c
@@ -587,12 +587,13 @@ Ext4GetPosition (
@param[in] This A pointer to the EFI_FILE_PROTOCOL instance that is the
file handle to set the requested position on.
- @param[in] Position The byte position from the start of the file to set.
+ @param[in] Position The byte position from the start of the file to set.
- @retval EFI_SUCCESS The position was set.
- @retval EFI_UNSUPPORTED The seek request for nonzero is not valid on open
- directories.
- @retval EFI_DEVICE_ERROR An attempt was made to set the position of a deleted file.
+ @retval EFI_SUCCESS The position was set.
+ @retval EFI_INVALID_PARAMETER The seek request for non-zero position is not valid on open
+ directories.
+ @retval EFI_UNSUPPORTED The seek request for position is exceeds FileSize.
+ @retval EFI_DEVICE_ERROR An attempt was made to set the position of a deleted file.
**/
EFI_STATUS
@@ -603,17 +604,23 @@ Ext4SetPosition (
)
{
EXT4_FILE *File;
+ UINT64 FileSize;
File = EXT4_FILE_FROM_THIS (This);
// Only seeks to 0 (so it resets the ReadDir operation) are allowed
if (Ext4FileIsDir (File) && (Position != 0)) {
- return EFI_UNSUPPORTED;
+ return EFI_INVALID_PARAMETER;
}
+ FileSize = EXT4_INODE_SIZE (File->Inode);
+
// -1 (0xffffff.......) seeks to the end of the file
if (Position == (UINT64)-1) {
- Position = EXT4_INODE_SIZE (File->Inode);
+ Position = FileSize;
+ } else if (Position > FileSize) {
+ DEBUG ((DEBUG_FS, "[ext4] Ext4SetPosition Cannot seek to #%Lx of %Lx\n", Position, FileSize));
+ return EFI_UNSUPPORTED;
}
File->Position = Position;
--
2.38.1
next prev parent reply other threads:[~2022-12-09 16:11 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-09 16:10 [edk2-platforms][PATCH v1 00/12] Ext4Pkg: Code correctness and security improvements Savva Mitrofanov
2022-12-09 16:10 ` [edk2-platforms][PATCH v1 01/12] Ext4Pkg: Fix memory leak in Ext4RetrieveDirent Savva Mitrofanov
2022-12-09 16:10 ` [edk2-platforms][PATCH v1 02/12] Ext4Pkg: Move EXT4_NAME_MAX definition to Ext4Disk.h Savva Mitrofanov
2022-12-09 16:10 ` [edk2-platforms][PATCH v1 03/12] Ext4Pkg: Fix global buffer overflow in Ext4ReadDir Savva Mitrofanov
2022-12-09 16:10 ` [edk2-platforms][PATCH v1 04/12] Ext4Pkg: Fix incorrect checksum metadata feature check Savva Mitrofanov
2022-12-09 16:10 ` [edk2-platforms][PATCH v1 05/12] Ext4Pkg: Fix division by zero by adding check for s_inodes_per_group Savva Mitrofanov
2022-12-09 16:10 ` Savva Mitrofanov [this message]
2022-12-09 22:12 ` [edk2-platforms][PATCH v1 06/12] Ext4Pkg: Add comparison between Position and FileSize in Ext4SetPosition Pedro Falcato
2022-12-12 11:44 ` Savva Mitrofanov
2022-12-09 16:10 ` [edk2-platforms][PATCH v1 07/12] Ext4Pkg: Add inode number validity check Savva Mitrofanov
2022-12-09 16:11 ` [edk2-platforms][PATCH v1 08/12] Ext4Pkg: Fix shift out of bounds in Ext4OpenSuperblock Savva Mitrofanov
2022-12-09 16:11 ` [edk2-platforms][PATCH v1 09/12] Ext4Pkg: Correct integer overflow check on multiplication in DiskUtil Savva Mitrofanov
2022-12-09 22:16 ` Pedro Falcato
2022-12-09 16:11 ` [edk2-platforms][PATCH v1 10/12] Ext4Pkg: Check that source file is directory in Ext4OpenInternal Savva Mitrofanov
2022-12-09 16:11 ` [edk2-platforms][PATCH v1 11/12] Ext4Pkg: Check VolumeName allocation correctness in Ext4GetVolumeName Savva Mitrofanov
2022-12-09 16:11 ` [edk2-platforms][PATCH v1 12/12] Ext4Pkg: Add missing exit Status in Ext4OpenDirent Savva Mitrofanov
2022-12-09 22:28 ` [edk2-platforms][PATCH v1 00/12] Ext4Pkg: Code correctness and security improvements Pedro Falcato
2022-12-12 14:40 ` Savva Mitrofanov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20221209161104.70220-7-savvamtr@gmail.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox