From 172b494fc64ec40020e79d432fd74e1a1759f13a Mon Sep 17 00:00:00 2001 From: Javier Martinez Canillas Date: Tue, 26 Nov 2019 10:28:55 +0100 Subject: [PATCH] grub-set-bootflag: Write new env to tmpfile and then rename Resolves: CVE-2019-14865 Resolves: rhbz#1776580 Signed-off-by: Javier Martinez Canillas --- ...g-Update-comment-about-running-as-ro.patch | 27 ++++ ...g-Write-new-env-to-tmpfile-and-then-.patch | 152 ++++++++++++++++++ grub.patches | 2 + grub2.spec | 7 +- 4 files changed, 187 insertions(+), 1 deletion(-) create mode 100644 0190-grub-set-bootflag-Update-comment-about-running-as-ro.patch create mode 100644 0191-grub-set-bootflag-Write-new-env-to-tmpfile-and-then-.patch diff --git a/0190-grub-set-bootflag-Update-comment-about-running-as-ro.patch b/0190-grub-set-bootflag-Update-comment-about-running-as-ro.patch new file mode 100644 index 0000000..cd4ef77 --- /dev/null +++ b/0190-grub-set-bootflag-Update-comment-about-running-as-ro.patch @@ -0,0 +1,27 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Hans de Goede +Date: Wed, 13 Nov 2019 12:15:43 +0100 +Subject: [PATCH] grub-set-bootflag: Update comment about running as root + through pkexec + +We have stopped using pkexec for grub-set-bootflag, instead it is now +installed suid root, update the comment accordingly. + +Signed-off-by: Hans de Goede +--- + util/grub-set-bootflag.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/util/grub-set-bootflag.c b/util/grub-set-bootflag.c +index 6a79ee67444..65d74ce010f 100644 +--- a/util/grub-set-bootflag.c ++++ b/util/grub-set-bootflag.c +@@ -18,7 +18,7 @@ + */ + + /* +- * NOTE this gets run by users as root (through pkexec), so this does not ++ * NOTE this gets run by users as root (its suid root), so this does not + * use any grub library / util functions to allow for easy auditing. + * The grub headers are only included to get certain defines. + */ diff --git a/0191-grub-set-bootflag-Write-new-env-to-tmpfile-and-then-.patch b/0191-grub-set-bootflag-Write-new-env-to-tmpfile-and-then-.patch new file mode 100644 index 0000000..122bf68 --- /dev/null +++ b/0191-grub-set-bootflag-Write-new-env-to-tmpfile-and-then-.patch @@ -0,0 +1,152 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Hans de Goede +Date: Wed, 13 Nov 2019 13:02:01 +0100 +Subject: [PATCH] grub-set-bootflag: Write new env to tmpfile and then rename + +Make the grubenv writing code in grub-set-bootflag more robust by +writing the modified grubenv to a tmpfile first and then renaming the +tmpfile over the old grubenv (following symlinks). + +Signed-off-by: Hans de Goede +--- + util/grub-set-bootflag.c | 87 +++++++++++++++++++++++++++++++++++++++++++----- + 1 file changed, 78 insertions(+), 9 deletions(-) + +diff --git a/util/grub-set-bootflag.c b/util/grub-set-bootflag.c +index 65d74ce010f..d1c5e28862b 100644 +--- a/util/grub-set-bootflag.c ++++ b/util/grub-set-bootflag.c +@@ -28,7 +28,9 @@ + #include + #include /* For GRUB_ENVBLK_DEFCFG define */ + #include ++#include + #include ++#include + #include + #include + +@@ -54,8 +56,10 @@ 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; ++ /* +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, len, ret; ++ int i, fd, len, ret; + FILE *f; + + if (argc != 2) +@@ -77,7 +81,32 @@ int main(int argc, char *argv[]) + bootflag = bootflags[i]; + len = strlen (bootflag); + +- f = fopen (GRUBENV, "r"); ++ /* ++ * 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. ++ */ ++ ret = setuid(0); ++ if (ret) ++ { ++ perror ("Error setuid(0) failed"); ++ return 1; ++ } ++ ++ ret = setgid(0); ++ if (ret) ++ { ++ perror ("Error setgid(0) failed"); ++ return 1; ++ } ++ ++ /* Canonicalize GRUBENV filename, resolving symlinks, etc. */ ++ if (!realpath(GRUBENV, env_filename)) ++ { ++ perror ("Error canonicalizing " GRUBENV " filename"); ++ return 1; ++ } ++ ++ f = fopen (env_filename, "r"); + if (!f) + { + perror ("Error opening " GRUBENV " for reading"); +@@ -132,30 +161,70 @@ int main(int argc, char *argv[]) + snprintf(buf, sizeof(buf), "%s=1\n", bootflag); + memcpy(s, buf, len + 3); + +- /* "r+", don't truncate so that the diskspace stays reserved */ +- f = fopen (GRUBENV, "r+"); ++ ++ /* ++ * 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. ++ */ ++ snprintf(tmp_filename, sizeof(tmp_filename), "%sXXXXXX", env_filename); ++ fd = mkstemp(tmp_filename); ++ if (fd == -1) ++ { ++ perror ("Creating tmpfile failed"); ++ return 1; ++ } ++ ++ f = fdopen (fd, "w"); + if (!f) + { +- perror ("Error opening " GRUBENV " for writing"); ++ perror ("Error fdopen of tmpfile failed"); ++ unlink(tmp_filename); + return 1; + } + + ret = fwrite (env, 1, GRUBENV_SIZE, f); + if (ret != GRUBENV_SIZE) + { +- perror ("Error writing to " GRUBENV); ++ perror ("Error writing tmpfile"); ++ unlink(tmp_filename); + return 1; + } + + ret = fflush (f); + if (ret) + { +- perror ("Error flushing " GRUBENV); ++ perror ("Error flushing tmpfile"); ++ unlink(tmp_filename); + return 1; + } + +- fsync (fileno (f)); +- fclose (f); ++ ret = fsync (fileno (f)); ++ if (ret) ++ { ++ perror ("Error syncing tmpfile"); ++ unlink(tmp_filename); ++ return 1; ++ } ++ ++ ret = fclose (f); ++ if (ret) ++ { ++ perror ("Error closing tmpfile"); ++ unlink(tmp_filename); ++ return 1; ++ } ++ ++ /* ++ * 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). ++ */ ++ ret = rename(tmp_filename, env_filename); ++ if (ret) ++ { ++ perror ("Error renaming tmpfile to " GRUBENV " failed"); ++ unlink(tmp_filename); ++ return 1; ++ } + + return 0; + } diff --git a/grub.patches b/grub.patches index c5a3b8d..1b8f6e9 100644 --- a/grub.patches +++ b/grub.patches @@ -187,3 +187,5 @@ Patch0186: 0186-grub-core-loader-efi-fdt.c-Do-not-copy-random-memory.patch Patch0187: 0187-linux-efi-arm-fdt-break-FDT-extra-allocation-space-o.patch Patch0188: 0188-Don-t-add-a-class-option-to-menu-entries-generated-f.patch Patch0189: 0189-blscfg-Fix-typo-for-gfxpayload-variable-name.patch +Patch0190: 0190-grub-set-bootflag-Update-comment-about-running-as-ro.patch +Patch0191: 0191-grub-set-bootflag-Write-new-env-to-tmpfile-and-then-.patch diff --git a/grub2.spec b/grub2.spec index ca2b4d2..da1834e 100644 --- a/grub2.spec +++ b/grub2.spec @@ -7,7 +7,7 @@ Name: grub2 Epoch: 1 Version: 2.02 -Release: 101%{?dist} +Release: 102%{?dist} Summary: Bootloader with support for Linux, Multiboot and more License: GPLv3+ URL: http://www.gnu.org/software/grub/ @@ -518,6 +518,11 @@ rm -r /boot/grub2.tmp/ || : %endif %changelog +* Tue Nov 26 2019 Javier Martinez Canillas - 2.02-102 +- grub-set-bootflag: Write new env to tmpfile and then rename (hdegoede) + Resolves: CVE-2019-14865 + Resolves: rhbz#1776580 + * Wed Oct 16 2019 Javier Martinez Canillas - 2.02-101 - 99-grub-mkconfig: Also disable BLS usage for Xen Dom0 hosts Resolves: rhbz#1761799