diff --git a/1005-CVE-2024-1048-grub-set-bootflag-Conservative-partial-fix.patch b/1005-CVE-2024-1048-grub-set-bootflag-Conservative-partial-fix.patch new file mode 100644 index 0000000000000000000000000000000000000000..4b9a81d156a2b1ab7705d29f1d76dbaee60c218f --- /dev/null +++ b/1005-CVE-2024-1048-grub-set-bootflag-Conservative-partial-fix.patch @@ -0,0 +1,154 @@ +From 13bba93bc3c2031199356810792f18f969589cab Mon Sep 17 00:00:00 2001 +From: Solar Designer +Date: Tue, 6 Feb 2024 21:39:41 +0100 +Subject: [PATCH 1/3] 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. + +Reference:https://src.fedoraproject.org/rpms/grub2/c/de8520b84a00acd5152bfacb433cc577fe825bca?branch=rawhide +Conflict:NA + +Signed-off-by: Solar Designer +--- + util/grub-set-bootflag.c | 30 +++++++++++++++++------------- + 1 file changed, 17 insertions(+), 13 deletions(-) + +diff --git a/util/grub-set-bootflag.c b/util/grub-set-bootflag.c +index d1c5e28..6b2561c 100644 +--- a/util/grub-set-bootflag.c ++++ b/util/grub-set-bootflag.c +@@ -33,6 +33,8 @@ + #include + #include + #include ++#include ++#include + + #define GRUBENV "/" GRUB_BOOT_DIR_NAME "/" GRUB_DIR_NAME "/" GRUB_ENVBLK_DEFCFG + #define GRUBENV_SIZE 1024 +@@ -55,12 +57,17 @@ static void usage(void) + 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) + { +@@ -82,20 +89,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) +- { +- perror ("Error setuid(0) failed"); +- return 1; +- } +- +- ret = setgid(0); +- if (ret) ++ if (setegid(0)) + { +- perror ("Error setgid(0) failed"); ++ perror ("Error setegid(0) failed"); + return 1; + } + +@@ -124,6 +122,10 @@ 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))) + { +@@ -159,6 +161,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); + + +-- +2.39.3 + diff --git a/1006-CVE-2024-1048-grub-set-bootflag-More-complete-fix.patch b/1006-CVE-2024-1048-grub-set-bootflag-More-complete-fix.patch new file mode 100644 index 0000000000000000000000000000000000000000..dddefec00a829f5d12cd6902d6ff22b54b11f981 --- /dev/null +++ b/1006-CVE-2024-1048-grub-set-bootflag-More-complete-fix.patch @@ -0,0 +1,190 @@ +From 9d6d08487e972d1e2b558117c297032741e0260b Mon Sep 17 00:00:00 2001 +From: Solar Designer +Date: Tue, 6 Feb 2024 21:56:21 +0100 +Subject: [PATCH 2/3] 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. + +Reference:https://src.fedoraproject.org/rpms/grub2/c/de8520b84a00acd5152bfacb433cc577fe825bca?branch=rawhide +Conflict:NA + +Signed-off-by: Solar Designer +--- + util/grub-set-bootflag.c | 87 ++++++++++++++++++++++++++++++++++------ + 1 file changed, 75 insertions(+), 12 deletions(-) + +diff --git a/util/grub-set-bootflag.c b/util/grub-set-bootflag.c +index 6b2561c..467ba7f 100644 +--- a/util/grub-set-bootflag.c ++++ b/util/grub-set-bootflag.c +@@ -33,6 +33,7 @@ + #include + #include + #include ++#include + #include + #include + +@@ -58,15 +59,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) +@@ -93,7 +91,7 @@ int main(int argc, char *argv[]) + */ + if (setegid(0)) + { +- perror ("Error setegid(0) failed"); ++ perror ("setegid(0) failed"); + return 1; + } + +@@ -165,19 +163,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) + { +@@ -202,23 +263,25 @@ int main(int argc, char *argv[]) + return 1; + } + +- ret = fsync (fileno (f)); ++ ret = ftruncate (fileno (f), GRUBENV_SIZE); + if (ret) + { +- perror ("Error syncing tmpfile"); ++ perror ("Error truncating tmpfile"); + unlink(tmp_filename); + return 1; + } + +- ret = fclose (f); ++ ret = fsync (fileno (f)); + if (ret) + { +- perror ("Error closing tmpfile"); ++ perror ("Error syncing 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). + */ +-- +2.39.3 + diff --git a/1007-CVE-2024-1048-grub-set-bootflag-Exit-calmly-when-not.patch b/1007-CVE-2024-1048-grub-set-bootflag-Exit-calmly-when-not.patch new file mode 100644 index 0000000000000000000000000000000000000000..7119c350f74bb35e1e683f2ddeef854aa865d4c6 --- /dev/null +++ b/1007-CVE-2024-1048-grub-set-bootflag-Exit-calmly-when-not.patch @@ -0,0 +1,42 @@ +From 4b25aec803d53de072db9b04d60a5d4e94c29d58 Mon Sep 17 00:00:00 2001 +From: Solar Designer +Date: Tue, 6 Feb 2024 22:05:45 +0100 +Subject: [PATCH 3/3] 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. + +Reference:https://src.fedoraproject.org/rpms/grub2/c/de8520b84a00acd5152bfacb433cc577fe825bca?branch=rawhide +Conflict:NA + +Signed-off-by: Solar Designer +--- + 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 467ba7f..e42daac 100644 +--- a/util/grub-set-bootflag.c ++++ b/util/grub-set-bootflag.c +@@ -86,6 +86,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. + */ +-- +2.39.3 + diff --git a/grub.patches b/grub.patches index 76d48cf58f5af137cee71faf04dcc81f6e37cb47..cf788bc2b782c846721b059c890018ec4089f10f 100644 --- a/grub.patches +++ b/grub.patches @@ -202,3 +202,6 @@ Patch1001: 1000-drop-other-efi_call_X-and-fix-build-issue.patch Patch1002: 0001-newfeature-tpcm-add-tpcm-support.patch Patch1003: 0002-newfeature-tpcm-embed-tpcm-module-into-kernel.img-by.patch Patch1004: 1004-Revert-Add-support-for-Linux-EFI-stub-loading.patch +Patch1005: 1005-CVE-2024-1048-grub-set-bootflag-Conservative-partial-fix.patch +Patch1006: 1006-CVE-2024-1048-grub-set-bootflag-More-complete-fix.patch +Patch1007: 1007-CVE-2024-1048-grub-set-bootflag-Exit-calmly-when-not.patch diff --git a/grub2.spec b/grub2.spec index b74235800a6b634045966f58a09b8e62647dd528..50c9a32a9225b951a08fb5bebd68aa51541d23e7 100644 --- a/grub2.spec +++ b/grub2.spec @@ -1,4 +1,4 @@ -%define anolis_release 4 +%define anolis_release 5 %global _lto_cflags %{nil} %undefine _hardened_build @@ -506,6 +506,9 @@ mv ${EFI_HOME}/grub.cfg.stb ${EFI_HOME}/grub.cfg %endif %changelog +* Wed May 15 2024 Kaiqiang Wang - 2.12-5 +- fix CVE-2024-1048 + * Mon Apr 22 2024 Chang Gao - 2.12-4 - Revert efi patch on loongarch