147 lines
5.6 KiB
Diff
147 lines
5.6 KiB
Diff
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);
|
|
|
|
|