Sync with rhel9 for critical patches

Resolves: #RHEL-56733
Signed-off-by: Nicolas Frayer <nfrayer@redhat.com>
This commit is contained in:
Nicolas Frayer 2024-08-30 11:57:29 +02:00
parent 7e8f0f0dcf
commit 6fd4bccf50
13 changed files with 1073 additions and 1 deletions

View File

@ -0,0 +1,146 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Solar Designer <solar@openwall.com>
Date: Tue, 6 Feb 2024 21:39:41 +0100
Subject: [PATCH] grub-set-bootflag: Conservative partial fix for CVE-2024-1048
Following up on CVE-2019-14865 and taking a fresh look at
grub2-set-bootflag now (through my work at CIQ on Rocky Linux), I saw some
other ways in which users could still abuse this little program:
1. After CVE-2019-14865 fix, grub2-set-bootflag no longer rewrites the
grubenv file in-place, but writes into a temporary file and renames it
over the original, checking for error returns from each call first.
This prevents the original file truncation vulnerability, but it can
leave the temporary file around if the program is killed before it can
rename or remove the file. There are still many ways to get the program
killed, such as through RLIMIT_FSIZE triggering SIGXFSZ (tested,
reliable) or by careful timing (tricky) of signals sent by process group
leader, pty, pre-scheduled timers, SIGXCPU (probably not an exhaustive
list). Invoking the program multiple times fills up /boot (or if /boot
is not separate, then it can fill up the root filesystem). Since the
files are tiny, the filesystem is likely to run out of free inodes
before it'd run out of blocks, but the effect is similar - can't create
new files after this point (but still can add data to existing files,
such as logs).
2. After CVE-2019-14865 fix, grub2-set-bootflag naively tries to protect
itself from signals by becoming full root. (This does protect it from
signals sent by the user directly to the PID, but e.g. "kill -9 -1" by
the user still works.) A side effect of such "protection" is that it's
possible to invoke more concurrent instances of grub2-set-bootflag than
the user's RLIMIT_NPROC would normally permit (as specified e.g. in
/etc/security/limits.conf, or say in Apache httpd's RLimitNPROC if
grub2-set-bootflag would be abused by a website script), thereby
exhausting system resources (e.g., bypassing RAM usage limit if
RLIMIT_AS was also set).
3. umask is inherited. Again, due to how the CVE-2019-14865 fix creates
a new file, and due to how mkstemp() works, this affects grubenv's new
file permissions. Luckily, mkstemp() forces them to be no more relaxed
than 0600, but the user ends up being able to set them e.g. to 0.
Luckily, at least in my testing GRUB still works fine even when the file
has such (lack of) permissions.
This commit deals with the abuses above as follows:
1. RLIMIT_FSIZE is pre-checked, so this specific way to get the process
killed should no longer work. However, this isn't a complete fix
because there are other ways to get the process killed after it has
created the temporary file.
The commit also fixes bug 1975892 ("RFE: grub2-set-bootflag should not
write the grubenv when the flag being written is already set") and
similar for "menu_show_once", which further reduces the abuse potential.
2. RLIMIT_NPROC bypass should be avoided by not becoming full root (aka
dropping the partial "kill protection").
3. A safe umask is set.
This is a partial fix (temporary files can still accumulate, but this is
harder to trigger).
While at it, this commit also fixes potential 1- or 2-byte over-read of
env[] if its content is malformed - this was not a security issue since the
grubenv file is trusted input, and the fix is just for robustness.
Signed-off-by: Solar Designer <solar@openwall.com>
---
util/grub-set-bootflag.c | 29 ++++++++++++++++-------------
1 file changed, 16 insertions(+), 13 deletions(-)
diff --git a/util/grub-set-bootflag.c b/util/grub-set-bootflag.c
index 3b4c25ca2ac6..5bbbef804391 100644
--- a/util/grub-set-bootflag.c
+++ b/util/grub-set-bootflag.c
@@ -33,6 +33,8 @@
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
+#include <sys/stat.h>
+#include <sys/resource.h>
#include "progname.h"
@@ -57,12 +59,17 @@ static void usage(FILE *out)
int main(int argc, char *argv[])
{
/* NOTE buf must be at least the longest bootflag length + 4 bytes */
- char env[GRUBENV_SIZE + 1], buf[64], *s;
+ char env[GRUBENV_SIZE + 1 + 2], buf[64], *s;
/* +1 for 0 termination, +6 for "XXXXXX" in tmp filename */
char env_filename[PATH_MAX + 1], tmp_filename[PATH_MAX + 6 + 1];
const char *bootflag;
int i, fd, len, ret;
FILE *f;
+ struct rlimit rlim;
+
+ if (getrlimit(RLIMIT_FSIZE, &rlim) || rlim.rlim_cur < GRUBENV_SIZE || rlim.rlim_max < GRUBENV_SIZE)
+ return 1;
+ umask(077);
if (argc != 2)
{
@@ -94,20 +101,11 @@ int main(int argc, char *argv[])
len = strlen (bootflag);
/*
- * Really become root. setuid avoids an user killing us, possibly leaking
- * the tmpfile. setgid avoids the new grubenv's gid being that of the user.
+ * setegid avoids the new grubenv's gid being that of the user.
*/
- ret = setuid(0);
- if (ret)
+ if (setegid(0))
{
- perror ("Error setuid(0) failed");
- return 1;
- }
-
- ret = setgid(0);
- if (ret)
- {
- perror ("Error setgid(0) failed");
+ perror ("Error setegid(0) failed");
return 1;
}
@@ -136,6 +134,9 @@ int main(int argc, char *argv[])
/* 0 terminate env */
env[GRUBENV_SIZE] = 0;
+ /* not a valid flag value */
+ env[GRUBENV_SIZE + 1] = 0;
+ env[GRUBENV_SIZE + 2] = 0;
if (strncmp (env, GRUB_ENVBLK_SIGNATURE, strlen (GRUB_ENVBLK_SIGNATURE)))
{
@@ -171,6 +172,8 @@ int main(int argc, char *argv[])
/* The grubenv is not 0 terminated, so memcpy the name + '=' , '1', '\n' */
snprintf(buf, sizeof(buf), "%s=1\n", bootflag);
+ if (!memcmp(s, buf, len + 3))
+ return 0; /* nothing to do */
memcpy(s, buf, len + 3);

View File

@ -0,0 +1,187 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Solar Designer <solar@openwall.com>
Date: Tue, 6 Feb 2024 21:56:21 +0100
Subject: [PATCH] grub-set-bootflag: More complete fix for CVE-2024-1048
Switch to per-user fixed temporary filenames along with a weird locking
mechanism, which is explained in source code comments. This is a more
complete fix than the previous commit (temporary files can't accumulate).
Unfortunately, it introduces new risks (by working on a temporary file
shared between the user's invocations), which are _hopefully_ avoided by
the patch's elaborate logic. I actually got it wrong at first, which
suggests that this logic is hard to reason about, and more errors or
omissions are possible. It also relies on the kernel's primitives' exact
semantics to a greater extent (nothing out of the ordinary, though).
Remaining issues that I think cannot reasonably be fixed without a
redesign (e.g., having per-flag files with nothing else in them) and
without introducing new issues:
A. A user can still revert a concurrent user's attempt of setting the
other flag - or of making other changes to grubenv by means other than
this program.
B. One leftover temporary file per user is still possible.
Signed-off-by: Solar Designer <solar@openwall.com>
---
util/grub-set-bootflag.c | 95 ++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 79 insertions(+), 16 deletions(-)
diff --git a/util/grub-set-bootflag.c b/util/grub-set-bootflag.c
index 5bbbef804391..514c4f9091ac 100644
--- a/util/grub-set-bootflag.c
+++ b/util/grub-set-bootflag.c
@@ -33,6 +33,7 @@
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
+#include <sys/file.h>
#include <sys/stat.h>
#include <sys/resource.h>
@@ -60,15 +61,12 @@ int main(int argc, char *argv[])
{
/* NOTE buf must be at least the longest bootflag length + 4 bytes */
char env[GRUBENV_SIZE + 1 + 2], buf[64], *s;
- /* +1 for 0 termination, +6 for "XXXXXX" in tmp filename */
- char env_filename[PATH_MAX + 1], tmp_filename[PATH_MAX + 6 + 1];
+ /* +1 for 0 termination, +11 for ".%u" in tmp filename */
+ char env_filename[PATH_MAX + 1], tmp_filename[PATH_MAX + 11 + 1];
const char *bootflag;
int i, fd, len, ret;
FILE *f;
- struct rlimit rlim;
- if (getrlimit(RLIMIT_FSIZE, &rlim) || rlim.rlim_cur < GRUBENV_SIZE || rlim.rlim_max < GRUBENV_SIZE)
- return 1;
umask(077);
if (argc != 2)
@@ -105,7 +103,7 @@ int main(int argc, char *argv[])
*/
if (setegid(0))
{
- perror ("Error setegid(0) failed");
+ perror ("setegid(0) failed");
return 1;
}
@@ -176,19 +174,82 @@ int main(int argc, char *argv[])
return 0; /* nothing to do */
memcpy(s, buf, len + 3);
+ struct rlimit rlim;
+ if (getrlimit(RLIMIT_FSIZE, &rlim) || rlim.rlim_cur < GRUBENV_SIZE || rlim.rlim_max < GRUBENV_SIZE)
+ {
+ fprintf (stderr, "Resource limits undetermined or too low\n");
+ return 1;
+ }
+
+ /*
+ * Here we work under the premise that we shouldn't write into the target
+ * file directly because we might not be able to have all of our changes
+ * written completely and atomically. That was CVE-2019-14865, known to
+ * have been triggerable via RLIMIT_FSIZE. While we've dealt with that
+ * specific attack via the check above, there may be other possibilities.
+ */
/*
* Create a tempfile for writing the new env. Use the canonicalized filename
* for the template so that the tmpfile is in the same dir / on same fs.
+ *
+ * We now use per-user fixed temporary filenames, so that a user cannot cause
+ * multiple files to accumulate.
+ *
+ * We don't use O_EXCL so that a stale temporary file doesn't prevent further
+ * usage of the program by the user.
*/
- snprintf(tmp_filename, sizeof(tmp_filename), "%sXXXXXX", env_filename);
- fd = mkstemp(tmp_filename);
+ snprintf(tmp_filename, sizeof(tmp_filename), "%s.%u", env_filename, getuid());
+ fd = open(tmp_filename, O_CREAT | O_WRONLY, 0600);
if (fd == -1)
{
perror ("Creating tmpfile failed");
return 1;
}
+ /*
+ * The lock prevents the same user from reaching further steps ending in
+ * rename() concurrently, in which case the temporary file only partially
+ * written by one invocation could be renamed to the target file by another.
+ *
+ * The lock also guards the slow fsync() from concurrent calls. After the
+ * first time that and the rename() complete, further invocations for the
+ * same flag become no-ops.
+ *
+ * We lock the temporary file rather than the target file because locking the
+ * latter would allow any user having SIGSTOP'ed their process to make all
+ * other users' invocations fail (or lock up if we'd use blocking mode).
+ *
+ * We use non-blocking mode (LOCK_NB) because the lock having been taken by
+ * another process implies that the other process would normally have already
+ * renamed the file to target by the time it releases the lock (and we could
+ * acquire it), so we'd be working directly on the target if we proceeded,
+ * which is undesirable, and we'd kind of fail on the already-done rename.
+ */
+ if (flock(fd, LOCK_EX | LOCK_NB))
+ {
+ perror ("Locking tmpfile failed");
+ return 1;
+ }
+
+ /*
+ * Deal with the potential that another invocation proceeded all the way to
+ * rename() and process exit while we were between open() and flock().
+ */
+ {
+ struct stat st1, st2;
+ if (fstat(fd, &st1) || stat(tmp_filename, &st2))
+ {
+ perror ("stat of tmpfile failed");
+ return 1;
+ }
+ if (st1.st_dev != st2.st_dev || st1.st_ino != st2.st_ino)
+ {
+ fprintf (stderr, "Another invocation won race\n");
+ return 1;
+ }
+ }
+
f = fdopen (fd, "w");
if (!f)
{
@@ -213,6 +274,14 @@ int main(int argc, char *argv[])
return 1;
}
+ ret = ftruncate (fileno (f), GRUBENV_SIZE);
+ if (ret)
+ {
+ perror ("Error truncating tmpfile");
+ unlink(tmp_filename);
+ return 1;
+ }
+
ret = fsync (fileno (f));
if (ret)
{
@@ -221,15 +290,9 @@ int main(int argc, char *argv[])
return 1;
}
- ret = fclose (f);
- if (ret)
- {
- perror ("Error closing tmpfile");
- unlink(tmp_filename);
- return 1;
- }
-
/*
+ * We must not close the file before rename() as that would remove the lock.
+ *
* And finally rename the tmpfile with the new env over the old env, the
* linux kernel guarantees that this is atomic (from a syscall pov).
*/

View File

@ -0,0 +1,36 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Solar Designer <solar@openwall.com>
Date: Tue, 6 Feb 2024 22:05:45 +0100
Subject: [PATCH] grub-set-bootflag: Exit calmly when not running as root
Exit calmly when not installed SUID root and invoked by non-root. This
allows installing user/grub-boot-success.service unconditionally while
supporting non-SUID installation of the program for some limited usage.
Signed-off-by: Solar Designer <solar@openwall.com>
---
util/grub-set-bootflag.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/util/grub-set-bootflag.c b/util/grub-set-bootflag.c
index 514c4f9091ac..31a868aeca8a 100644
--- a/util/grub-set-bootflag.c
+++ b/util/grub-set-bootflag.c
@@ -98,6 +98,17 @@ int main(int argc, char *argv[])
bootflag = bootflags[i];
len = strlen (bootflag);
+ /*
+ * Exit calmly when not installed SUID root and invoked by non-root. This
+ * allows installing user/grub-boot-success.service unconditionally while
+ * supporting non-SUID installation of the program for some limited usage.
+ */
+ if (geteuid())
+ {
+ printf ("grub-set-bootflag not running as root, no action taken\n");
+ return 0;
+ }
+
/*
* setegid avoids the new grubenv's gid being that of the user.
*/

View File

@ -0,0 +1,90 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Maxim Suhanov <dfirblog@gmail.com>
Date: Mon, 28 Aug 2023 16:31:57 +0300
Subject: [PATCH] fs/ntfs: Fix an OOB write when parsing the $ATTRIBUTE_LIST
attribute for the $MFT file
When parsing an extremely fragmented $MFT file, i.e., the file described
using the $ATTRIBUTE_LIST attribute, current NTFS code will reuse a buffer
containing bytes read from the underlying drive to store sector numbers,
which are consumed later to read data from these sectors into another buffer.
These sectors numbers, two 32-bit integers, are always stored at predefined
offsets, 0x10 and 0x14, relative to first byte of the selected entry within
the $ATTRIBUTE_LIST attribute. Usually, this won't cause any problem.
However, when parsing a specially-crafted file system image, this may cause
the NTFS code to write these integers beyond the buffer boundary, likely
causing the GRUB memory allocator to misbehave or fail. These integers contain
values which are controlled by on-disk structures of the NTFS file system.
Such modification and resulting misbehavior may touch a memory range not
assigned to the GRUB and owned by firmware or another EFI application/driver.
This fix introduces checks to ensure that these sector numbers are never
written beyond the boundary.
Fixes: CVE-2023-4692
Reported-by: Maxim Suhanov <dfirblog@gmail.com>
Signed-off-by: Maxim Suhanov <dfirblog@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
grub-core/fs/ntfs.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/grub-core/fs/ntfs.c b/grub-core/fs/ntfs.c
index 3511e4e2cb6f..4681c7ac32a8 100644
--- a/grub-core/fs/ntfs.c
+++ b/grub-core/fs/ntfs.c
@@ -184,7 +184,7 @@ find_attr (struct grub_ntfs_attr *at, grub_uint8_t attr)
}
if (at->attr_end)
{
- grub_uint8_t *pa;
+ grub_uint8_t *pa, *pa_end;
at->emft_buf = grub_malloc (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR);
if (at->emft_buf == NULL)
@@ -209,11 +209,13 @@ find_attr (struct grub_ntfs_attr *at, grub_uint8_t attr)
}
at->attr_nxt = at->edat_buf;
at->attr_end = at->edat_buf + u32at (pa, 0x30);
+ pa_end = at->edat_buf + n;
}
else
{
at->attr_nxt = at->attr_end + u16at (pa, 0x14);
at->attr_end = at->attr_end + u32at (pa, 4);
+ pa_end = at->mft->buf + (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR);
}
at->flags |= GRUB_NTFS_AF_ALST;
while (at->attr_nxt < at->attr_end)
@@ -230,6 +232,13 @@ find_attr (struct grub_ntfs_attr *at, grub_uint8_t attr)
at->flags |= GRUB_NTFS_AF_GPOS;
at->attr_cur = at->attr_nxt;
pa = at->attr_cur;
+
+ if ((pa >= pa_end) || (pa_end - pa < 0x18))
+ {
+ grub_error (GRUB_ERR_BAD_FS, "can\'t parse attribute list");
+ return NULL;
+ }
+
grub_set_unaligned32 ((char *) pa + 0x10,
grub_cpu_to_le32 (at->mft->data->mft_start));
grub_set_unaligned32 ((char *) pa + 0x14,
@@ -240,6 +249,13 @@ find_attr (struct grub_ntfs_attr *at, grub_uint8_t attr)
{
if (*pa != attr)
break;
+
+ if ((pa >= pa_end) || (pa_end - pa < 0x18))
+ {
+ grub_error (GRUB_ERR_BAD_FS, "can\'t parse attribute list");
+ return NULL;
+ }
+
if (read_attr
(at, pa + 0x10,
u32at (pa, 0x10) * (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR),

View File

@ -0,0 +1,55 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Maxim Suhanov <dfirblog@gmail.com>
Date: Mon, 28 Aug 2023 16:32:33 +0300
Subject: [PATCH] fs/ntfs: Fix an OOB read when reading data from the resident
$DATA attribute
When reading a file containing resident data, i.e., the file data is stored in
the $DATA attribute within the NTFS file record, not in external clusters,
there are no checks that this resident data actually fits the corresponding
file record segment.
When parsing a specially-crafted file system image, the current NTFS code will
read the file data from an arbitrary, attacker-chosen memory offset and of
arbitrary, attacker-chosen length.
This allows an attacker to display arbitrary chunks of memory, which could
contain sensitive information like password hashes or even plain-text,
obfuscated passwords from BS EFI variables.
This fix implements a check to ensure that resident data is read from the
corresponding file record segment only.
Fixes: CVE-2023-4693
Reported-by: Maxim Suhanov <dfirblog@gmail.com>
Signed-off-by: Maxim Suhanov <dfirblog@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
grub-core/fs/ntfs.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/grub-core/fs/ntfs.c b/grub-core/fs/ntfs.c
index 4681c7ac32a8..1949d48a494f 100644
--- a/grub-core/fs/ntfs.c
+++ b/grub-core/fs/ntfs.c
@@ -401,7 +401,18 @@ read_data (struct grub_ntfs_attr *at, grub_uint8_t *pa, grub_uint8_t *dest,
{
if (ofs + len > u32at (pa, 0x10))
return grub_error (GRUB_ERR_BAD_FS, "read out of range");
- grub_memcpy (dest, pa + u32at (pa, 0x14) + ofs, len);
+
+ if (u32at (pa, 0x10) > (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR))
+ return grub_error (GRUB_ERR_BAD_FS, "resident attribute too large");
+
+ if (pa >= at->mft->buf + (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR))
+ return grub_error (GRUB_ERR_BAD_FS, "resident attribute out of range");
+
+ if (u16at (pa, 0x14) + u32at (pa, 0x10) >
+ (grub_addr_t) at->mft->buf + (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR) - (grub_addr_t) pa)
+ return grub_error (GRUB_ERR_BAD_FS, "resident attribute out of range");
+
+ grub_memcpy (dest, pa + u16at (pa, 0x14) + ofs, len);
return 0;
}

View File

@ -0,0 +1,70 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Maxim Suhanov <dfirblog@gmail.com>
Date: Mon, 28 Aug 2023 16:33:17 +0300
Subject: [PATCH] fs/ntfs: Fix an OOB read when parsing directory entries from
resident and non-resident index attributes
This fix introduces checks to ensure that index entries are never read
beyond the corresponding directory index.
The lack of this check is a minor issue, likely not exploitable in any way.
Reported-by: Maxim Suhanov <dfirblog@gmail.com>
Signed-off-by: Maxim Suhanov <dfirblog@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
grub-core/fs/ntfs.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/grub-core/fs/ntfs.c b/grub-core/fs/ntfs.c
index 1949d48a494f..72302033281a 100644
--- a/grub-core/fs/ntfs.c
+++ b/grub-core/fs/ntfs.c
@@ -599,7 +599,7 @@ get_utf8 (grub_uint8_t *in, grub_size_t len)
}
static int
-list_file (struct grub_ntfs_file *diro, grub_uint8_t *pos,
+list_file (struct grub_ntfs_file *diro, grub_uint8_t *pos, grub_uint8_t *end_pos,
grub_fshelp_iterate_dir_hook_t hook, void *hook_data)
{
grub_uint8_t *np;
@@ -610,6 +610,9 @@ list_file (struct grub_ntfs_file *diro, grub_uint8_t *pos,
grub_uint8_t namespace;
char *ustr;
+ if ((pos >= end_pos) || (end_pos - pos < 0x52))
+ break;
+
if (pos[0xC] & 2) /* end signature */
break;
@@ -617,6 +620,9 @@ list_file (struct grub_ntfs_file *diro, grub_uint8_t *pos,
ns = *(np++);
namespace = *(np++);
+ if (2 * ns > end_pos - pos - 0x52)
+ break;
+
/*
* Ignore files in DOS namespace, as they will reappear as Win32
* names.
@@ -802,7 +808,9 @@ grub_ntfs_iterate_dir (grub_fshelp_node_t dir,
}
cur_pos += 0x10; /* Skip index root */
- ret = list_file (mft, cur_pos + u16at (cur_pos, 0), hook, hook_data);
+ ret = list_file (mft, cur_pos + u16at (cur_pos, 0),
+ at->mft->buf + (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR),
+ hook, hook_data);
if (ret)
goto done;
@@ -889,6 +897,7 @@ grub_ntfs_iterate_dir (grub_fshelp_node_t dir,
(const grub_uint8_t *) "INDX")))
goto done;
ret = list_file (mft, &indx[0x18 + u16at (indx, 0x18)],
+ indx + (mft->data->idx_size << GRUB_NTFS_BLK_SHR),
hook, hook_data);
if (ret)
goto done;

View File

@ -0,0 +1,48 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Maxim Suhanov <dfirblog@gmail.com>
Date: Mon, 28 Aug 2023 16:33:44 +0300
Subject: [PATCH] fs/ntfs: Fix an OOB read when parsing bitmaps for index
attributes
This fix introduces checks to ensure that bitmaps for directory indices
are never read beyond their actual sizes.
The lack of this check is a minor issue, likely not exploitable in any way.
Reported-by: Maxim Suhanov <dfirblog@gmail.com>
Signed-off-by: Maxim Suhanov <dfirblog@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
grub-core/fs/ntfs.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/grub-core/fs/ntfs.c b/grub-core/fs/ntfs.c
index 72302033281a..74515114287f 100644
--- a/grub-core/fs/ntfs.c
+++ b/grub-core/fs/ntfs.c
@@ -839,6 +839,25 @@ grub_ntfs_iterate_dir (grub_fshelp_node_t dir,
if (is_resident)
{
+ if (bitmap_len > (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR))
+ {
+ grub_error (GRUB_ERR_BAD_FS, "resident bitmap too large");
+ goto done;
+ }
+
+ if (cur_pos >= at->mft->buf + (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR))
+ {
+ grub_error (GRUB_ERR_BAD_FS, "resident bitmap out of range");
+ goto done;
+ }
+
+ if (u16at (cur_pos, 0x14) + u32at (cur_pos, 0x10) >
+ (grub_addr_t) at->mft->buf + (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR) - (grub_addr_t) cur_pos)
+ {
+ grub_error (GRUB_ERR_BAD_FS, "resident bitmap out of range");
+ goto done;
+ }
+
grub_memcpy (bmp, cur_pos + u16at (cur_pos, 0x14),
bitmap_len);
}

View File

@ -0,0 +1,58 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Maxim Suhanov <dfirblog@gmail.com>
Date: Mon, 28 Aug 2023 16:38:19 +0300
Subject: [PATCH] fs/ntfs: Fix an OOB read when parsing a volume label
This fix introduces checks to ensure that an NTFS volume label is always
read from the corresponding file record segment.
The current NTFS code allows the volume label string to be read from an
arbitrary, attacker-chosen memory location. However, the bytes read are
always treated as UTF-16LE. So, the final string displayed is mostly
unreadable and it can't be easily converted back to raw bytes.
The lack of this check is a minor issue, likely not causing a significant
data leak.
Reported-by: Maxim Suhanov <dfirblog@gmail.com>
Signed-off-by: Maxim Suhanov <dfirblog@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
grub-core/fs/ntfs.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/grub-core/fs/ntfs.c b/grub-core/fs/ntfs.c
index 74515114287f..32ba8276dd8d 100644
--- a/grub-core/fs/ntfs.c
+++ b/grub-core/fs/ntfs.c
@@ -1209,13 +1209,29 @@ grub_ntfs_label (grub_device_t device, char **label)
init_attr (&mft->attr, mft);
pa = find_attr (&mft->attr, GRUB_NTFS_AT_VOLUME_NAME);
+
+ if (pa >= mft->buf + (mft->data->mft_size << GRUB_NTFS_BLK_SHR))
+ {
+ grub_error (GRUB_ERR_BAD_FS, "can\'t parse volume label");
+ goto fail;
+ }
+
+ if (mft->buf + (mft->data->mft_size << GRUB_NTFS_BLK_SHR) - pa < 0x16)
+ {
+ grub_error (GRUB_ERR_BAD_FS, "can\'t parse volume label");
+ goto fail;
+ }
+
if ((pa) && (pa[8] == 0) && (u32at (pa, 0x10)))
{
int len;
len = u32at (pa, 0x10) / 2;
pa += u16at (pa, 0x14);
- *label = get_utf8 (pa, len);
+ if (mft->buf + (mft->data->mft_size << GRUB_NTFS_BLK_SHR) - pa >= 2 * len)
+ *label = get_utf8 (pa, len);
+ else
+ grub_error (GRUB_ERR_BAD_FS, "can\'t parse volume label");
}
fail:

View File

@ -0,0 +1,156 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Maxim Suhanov <dfirblog@gmail.com>
Date: Mon, 28 Aug 2023 16:40:07 +0300
Subject: [PATCH] fs/ntfs: Make code more readable
Move some calls used to access NTFS attribute header fields into
functions with human-readable names.
Suggested-by: Daniel Kiper <daniel.kiper@oracle.com>
Signed-off-by: Maxim Suhanov <dfirblog@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
grub-core/fs/ntfs.c | 48 +++++++++++++++++++++++++++++++++---------------
1 file changed, 33 insertions(+), 15 deletions(-)
diff --git a/grub-core/fs/ntfs.c b/grub-core/fs/ntfs.c
index 32ba8276dd8d..991b1c2094f5 100644
--- a/grub-core/fs/ntfs.c
+++ b/grub-core/fs/ntfs.c
@@ -52,6 +52,24 @@ u64at (void *ptr, grub_size_t ofs)
return grub_le_to_cpu64 (grub_get_unaligned64 ((char *) ptr + ofs));
}
+static grub_uint16_t
+first_attr_off (void *mft_buf_ptr)
+{
+ return u16at (mft_buf_ptr, 0x14);
+}
+
+static grub_uint16_t
+res_attr_data_off (void *res_attr_ptr)
+{
+ return u16at (res_attr_ptr, 0x14);
+}
+
+static grub_uint32_t
+res_attr_data_len (void *res_attr_ptr)
+{
+ return u32at (res_attr_ptr, 0x10);
+}
+
grub_ntfscomp_func_t grub_ntfscomp_func;
static grub_err_t
@@ -106,7 +124,7 @@ init_attr (struct grub_ntfs_attr *at, struct grub_ntfs_file *mft)
{
at->mft = mft;
at->flags = (mft == &mft->data->mmft) ? GRUB_NTFS_AF_MMFT : 0;
- at->attr_nxt = mft->buf + u16at (mft->buf, 0x14);
+ at->attr_nxt = mft->buf + first_attr_off (mft->buf);
at->attr_end = at->emft_buf = at->edat_buf = at->sbuf = NULL;
}
@@ -154,7 +172,7 @@ find_attr (struct grub_ntfs_attr *at, grub_uint8_t attr)
return NULL;
}
- new_pos = &at->emft_buf[u16at (at->emft_buf, 0x14)];
+ new_pos = &at->emft_buf[first_attr_off (at->emft_buf)];
while (*new_pos != 0xFF)
{
if ((*new_pos == *at->attr_cur)
@@ -213,7 +231,7 @@ find_attr (struct grub_ntfs_attr *at, grub_uint8_t attr)
}
else
{
- at->attr_nxt = at->attr_end + u16at (pa, 0x14);
+ at->attr_nxt = at->attr_end + res_attr_data_off (pa);
at->attr_end = at->attr_end + u32at (pa, 4);
pa_end = at->mft->buf + (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR);
}
@@ -399,20 +417,20 @@ read_data (struct grub_ntfs_attr *at, grub_uint8_t *pa, grub_uint8_t *dest,
if (pa[8] == 0)
{
- if (ofs + len > u32at (pa, 0x10))
+ if (ofs + len > res_attr_data_len (pa))
return grub_error (GRUB_ERR_BAD_FS, "read out of range");
- if (u32at (pa, 0x10) > (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR))
+ if (res_attr_data_len (pa) > (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR))
return grub_error (GRUB_ERR_BAD_FS, "resident attribute too large");
if (pa >= at->mft->buf + (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR))
return grub_error (GRUB_ERR_BAD_FS, "resident attribute out of range");
- if (u16at (pa, 0x14) + u32at (pa, 0x10) >
+ if (res_attr_data_off (pa) + res_attr_data_len (pa) >
(grub_addr_t) at->mft->buf + (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR) - (grub_addr_t) pa)
return grub_error (GRUB_ERR_BAD_FS, "resident attribute out of range");
- grub_memcpy (dest, pa + u16at (pa, 0x14) + ofs, len);
+ grub_memcpy (dest, pa + res_attr_data_off (pa) + ofs, len);
return 0;
}
@@ -556,7 +574,7 @@ init_file (struct grub_ntfs_file *mft, grub_uint64_t mftno)
(unsigned long long) mftno);
if (!pa[8])
- mft->size = u32at (pa, 0x10);
+ mft->size = res_attr_data_len (pa);
else
mft->size = u64at (pa, 0x30);
@@ -801,7 +819,7 @@ grub_ntfs_iterate_dir (grub_fshelp_node_t dir,
(u32at (cur_pos, 0x18) != 0x490024) ||
(u32at (cur_pos, 0x1C) != 0x300033))
continue;
- cur_pos += u16at (cur_pos, 0x14);
+ cur_pos += res_attr_data_off (cur_pos);
if (*cur_pos != 0x30) /* Not filename index */
continue;
break;
@@ -830,7 +848,7 @@ grub_ntfs_iterate_dir (grub_fshelp_node_t dir,
{
int is_resident = (cur_pos[8] == 0);
- bitmap_len = ((is_resident) ? u32at (cur_pos, 0x10) :
+ bitmap_len = ((is_resident) ? res_attr_data_len (cur_pos) :
u32at (cur_pos, 0x28));
bmp = grub_malloc (bitmap_len);
@@ -851,14 +869,14 @@ grub_ntfs_iterate_dir (grub_fshelp_node_t dir,
goto done;
}
- if (u16at (cur_pos, 0x14) + u32at (cur_pos, 0x10) >
+ if (res_attr_data_off (cur_pos) + res_attr_data_len (cur_pos) >
(grub_addr_t) at->mft->buf + (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR) - (grub_addr_t) cur_pos)
{
grub_error (GRUB_ERR_BAD_FS, "resident bitmap out of range");
goto done;
}
- grub_memcpy (bmp, cur_pos + u16at (cur_pos, 0x14),
+ grub_memcpy (bmp, cur_pos + res_attr_data_off (cur_pos),
bitmap_len);
}
else
@@ -1222,12 +1240,12 @@ grub_ntfs_label (grub_device_t device, char **label)
goto fail;
}
- if ((pa) && (pa[8] == 0) && (u32at (pa, 0x10)))
+ if ((pa) && (pa[8] == 0) && (res_attr_data_len (pa)))
{
int len;
- len = u32at (pa, 0x10) / 2;
- pa += u16at (pa, 0x14);
+ len = res_attr_data_len (pa) / 2;
+ pa += res_attr_data_off (pa);
if (mft->buf + (mft->data->mft_size << GRUB_NTFS_BLK_SHR) - pa >= 2 * len)
*label = get_utf8 (pa, len);
else

View File

@ -0,0 +1,182 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Nicolas Frayer <nfrayer@redhat.com>
Date: Thu, 16 May 2024 10:58:32 +0200
Subject: [PATCH] cmd/search: Rework of CVE-2023-4001 fix
The initial fix implemented a new flag that forces the grub cfg
stub to be located on the same disk as grub. This created several
issues such as RAID machines not being able to boot as their
partition names under grub were different from the partition where
grub is located. It also simply means that any machines with the
/boot partition located on a disk other than the one containing grub
won't boot.
This commit denies booting if the grub cfg stub is located on a USB
drive with a duplicated UUID (UUID being the same as the partition
containing the actual grub cfg stub)
Signed-off-by: Nicolas Frayer <nfrayer@redhat.com>
---
grub-core/commands/search.c | 136 +++++++++++++++++++++++++++++++++++++++++---
1 file changed, 127 insertions(+), 9 deletions(-)
diff --git a/grub-core/commands/search.c b/grub-core/commands/search.c
index 819231751c38..1e61a5cf940f 100644
--- a/grub-core/commands/search.c
+++ b/grub-core/commands/search.c
@@ -30,6 +30,8 @@
#include <grub/i18n.h>
#include <grub/disk.h>
#include <grub/partition.h>
+#include <grub/efi/api.h>
+#include <grub/time.h>
GRUB_MOD_LICENSE ("GPLv3+");
@@ -54,6 +56,100 @@ struct search_ctx
int is_cache;
};
+static int
+is_device_usb (const char *name)
+{
+ int ret = 0;
+
+ grub_device_t dev = grub_device_open(name);
+
+ if (dev)
+ {
+ struct grub_efidisk_data
+ {
+ grub_efi_handle_t handle;
+ grub_efi_device_path_t *device_path;
+ grub_efi_device_path_t *last_device_path;
+ grub_efi_block_io_t *block_io;
+ struct grub_efidisk_data *next;
+ };
+
+ if (dev->disk && dev->disk->data)
+ {
+ struct grub_efidisk_data *dp = dev->disk->data;
+
+ if ( GRUB_EFI_DEVICE_PATH_TYPE (dp->last_device_path) == GRUB_EFI_MESSAGING_DEVICE_PATH_TYPE &&
+ GRUB_EFI_DEVICE_PATH_SUBTYPE (dp->last_device_path) == GRUB_EFI_USB_DEVICE_PATH_SUBTYPE)
+ {
+ ret = 1;
+ }
+ }
+ grub_device_close(dev);
+ }
+
+ return ret;
+}
+
+static int
+get_device_uuid(const char *name, char** quid)
+{
+ int ret = 0;
+
+ grub_device_t dev_part = grub_device_open(name);
+
+ if (dev_part)
+ {
+ grub_fs_t fs;
+
+ fs = grub_fs_probe (dev_part);
+
+#ifdef DO_SEARCH_FS_UUID
+#define read_fn fs_uuid
+#else
+#define read_fn fs_label
+#endif
+ if (fs && fs->read_fn)
+ {
+ fs->read_fn (dev_part, quid);
+
+ if (grub_errno == GRUB_ERR_NONE && *quid)
+ {
+ ret = 1;
+ }
+
+ }
+ grub_device_close (dev_part);
+ }
+
+ return ret;
+}
+struct uuid_context {
+ char* name;
+ char* uuid;
+};
+
+static int
+check_for_duplicate (const char *name, void *data)
+{
+ int ret = 0;
+ struct uuid_context * uuid_ctx = (struct uuid_context *)data;
+ char *quid = 0;
+
+ get_device_uuid(name, &quid);
+
+ if (quid == NULL)
+ return 0;
+
+ if (!grub_strcasecmp(quid, uuid_ctx->uuid) && grub_strcasecmp(name, uuid_ctx->name))
+ {
+ ret = 1;
+ }
+
+ grub_free(quid);
+
+ return ret;
+}
+
/* Helper for FUNC_NAME. */
static int
iterate_device (const char *name, void *data)
@@ -104,15 +200,37 @@ iterate_device (const char *name, void *data)
grub_str_sep (root_dev, root_disk, ',', rem_1);
grub_str_sep (name, name_disk, ',', rem_2);
if (root_disk != NULL && *root_disk != '\0' &&
- name_disk != NULL && *name_disk != '\0')
- if (grub_strcmp(root_disk, name_disk) != 0)
- {
- grub_free (root_disk);
- grub_free (name_disk);
- grub_free (rem_1);
- grub_free (rem_2);
- return 0;
- }
+ name_disk != NULL && *name_disk != '\0')
+ {
+ grub_device_t dev, dev_part;
+
+ if (is_device_usb(name) && !is_device_usb(root_dev))
+ {
+ char *quid_name = NULL;
+ int longlist = 0;
+ struct uuid_context uuid_ctx;
+ int ret = 0;
+
+ get_device_uuid(name, &quid_name);
+ if (!grub_strcmp(quid_name, ctx->key))
+ {
+ uuid_ctx.name = name;
+ uuid_ctx.uuid = quid_name;
+
+ ret = grub_device_iterate (check_for_duplicate, &uuid_ctx);
+
+ if (ret)
+ {
+ grub_printf("Duplicated media UUID found, rebooting ...\n");
+ grub_sleep(10);
+ grub_reboot();
+ }
+ }
+
+ if (quid_name) grub_free (quid_name);
+
+ }
+ }
}
grub_free (root_disk);
grub_free (name_disk);

View File

@ -0,0 +1,29 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Leo Sandoval <lsandova@redhat.com>
Date: Mon, 1 Jul 2024 12:52:13 -0600
Subject: [PATCH] grub-mkconfig.in: turn off executable owner bit
Stricker permissions are required on the grub.cfg file, resulting in
at most 0600 owner's file permissions. This resolves conflicting
requirement permissions on grub2-pc package's grub2.cfg file.
Resolves: RHEL-45870
Signed-off-by: Leo Sandoval <lsandova@redhat.com>
---
util/grub-mkconfig.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/util/grub-mkconfig.in b/util/grub-mkconfig.in
index 3f131eea2b12..8c2bb8259de1 100644
--- a/util/grub-mkconfig.in
+++ b/util/grub-mkconfig.in
@@ -328,7 +328,7 @@ and /etc/grub.d/* files or please file a bug report with
exit 1
else
# none of the children aborted with error, install the new grub.cfg
- oldumask=$(umask); umask 077
+ oldumask=$(umask); umask 177
cat ${grub_cfg}.new > ${grub_cfg}
umask $oldumask
rm -f ${grub_cfg}.new

View File

@ -353,3 +353,14 @@ Patch0352: 0352-grub2-mkconfig-Simplify-os_name-detection.patch
Patch0353: 0353-grub-mkconfig-Remove-check-for-mount-point-for-grub-.patch
Patch0354: 0354-grub-mkconfig-dont-overwrite-BLS-cmdline-if-BLSCFG.patch
Patch0355: 0355-grub2-mkconfig-Pass-all-boot-params-when-used-by-ana.patch
Patch0356: 0356-grub-set-bootflag-Conservative-partial-fix-for-CVE-2.patch
Patch0357: 0357-grub-set-bootflag-More-complete-fix-for-CVE-2024-104.patch
Patch0358: 0358-grub-set-bootflag-Exit-calmly-when-not-running-as-ro.patch
Patch0359: 0359-fs-ntfs-Fix-an-OOB-write-when-parsing-the-ATTRIBUTE_.patch
Patch0360: 0360-fs-ntfs-Fix-an-OOB-read-when-reading-data-from-the-r.patch
Patch0361: 0361-fs-ntfs-Fix-an-OOB-read-when-parsing-directory-entri.patch
Patch0362: 0362-fs-ntfs-Fix-an-OOB-read-when-parsing-bitmaps-for-ind.patch
Patch0363: 0363-fs-ntfs-Fix-an-OOB-read-when-parsing-a-volume-label.patch
Patch0364: 0364-fs-ntfs-Make-code-more-readable.patch
Patch0365: 0365-cmd-search-Rework-of-CVE-2023-4001-fix.patch
Patch0366: 0366-grub-mkconfig.in-turn-off-executable-owner-bit.patch

View File

@ -17,7 +17,7 @@
Name: grub2
Epoch: 1
Version: 2.06
Release: 126%{?dist}
Release: 127%{?dist}
Summary: Bootloader with support for Linux, Multiboot and more
License: GPL-3.0-or-later
URL: http://www.gnu.org/software/grub/
@ -573,6 +573,10 @@ mv ${EFI_HOME}/grub.cfg.stb ${EFI_HOME}/grub.cfg
%endif
%changelog
* Fri Aug 30 2024 Nicolas Frayer <nfrayer@redhat.com> - 2.06-127
- Sync with rhel9 for critical patches
- Resolves: #RHEL-56733
* Wed Aug 28 2024 Nicolas Frayer <nfrayer@redhat.com> - 2.06-126
- grub-mkconfig dont overwrite BLS cmdline if BLSCFG
- Resolves: #RHEL-53848