[SECURITY PATCH 000/117] Multiple GRUB2 vulnerabilities - 2021/03/02 round
Hi all, The BootHole vulnerability [1][2] announced last year encouraged many people to take a closer look at the security of boot process in general and the GRUB bootloader in particular. Due to that, during past few months we were getting reports of, and also discovering various security flaws in the GRUB ourselves. You can find the list of most severe ones which got CVEs assigned at the end of this message. The patch bundle fixing all these issues in the upstream GRUB contains 117 patches. In addition, we have been working on a generation number based revocation scheme termed UEFI Secure Boot Advanced Targeting (SBAT) [3]. This will require an UEFI dbx release and resigning all the artifacts -- shim, GRUB, kernel, etc. -- needed to boot the system. This is the same as we did for the BootHole series of vulnerabilities, but the SBAT work is designed to make this process much less painful in the future. Details of exactly what needs updating will be provided by the respective distros and vendors when updates become available. Here [4] we are listing at least some links to the messaging known at the time of this posting. It is important to know that shim and SBAT development is still ongoing. Full mitigation against all the CVEs will require an updated UEFI revocation list (dbx) which, in at least some cases, will not allow Secure Boot with today's boot artifacts. Vendor shims may explicitly permit known older boot artifacts to boot. At some stage, the dbx on new hardware will be updated. Updated GRUB2, shim and other boot artifacts from all the affected vendors will be made available when the embargo lifts or some time thereafter. An updated dbx from the various affected vendors will also ship, although possibly not at the same time. The new Microsoft dbx will be provided for download here [5]. I am posting all the GRUB2 upstream patches which fixes all security bugs found and reported up until now. Major Linux distros carry or will carry soon one form or another of these patches. Now all the GRUB2 upstream patches are in the GRUB2 git repository [6] too. In particular I would like to thank, in alphabetical order, the following people who were working really hard on the GRUB, shim and other things related to these issues: - Alexander Burmashev (Oracle), - Chris Co (Microsoft), - Chris Coulson (Canonical), - Cliff Perry (Red Hat), - Colin Watson (Debian), - D. Jared Dominguez (Red Hat), - Daniel Axtens (IBM), - Darren Kenny (Oracle), - Dimitri John Ledkov (Canonical), - Eric Snowberg (Oracle), - Ilja Van Sprundel (IOActive), - Ilya Okomin (Oracle), - Jan Setje-Eilers (Oracle), - Javier Martinez Canillas (Red Hat), - Jeremiah Cox (Microsoft), - John Haxby (Oracle), - Jordan Geurten (Microsoft), - Joseph Tartaro (IOActive), - Julian Andres Klode (Canonical), - Marco A Benatto (Red Hat), - Mario Limonciello (Dell), - Mathieu Trudel-Lapierre (Stack8), - Matthew Garrett (Google), - Máté Kukri, - Noam Rathaus (SSD Secure Disclosure), - NyankoSec (SSD Secure Disclosure), - Paulo Flabiano Smorigo (Canonical), - Peter Jones (Red Hat), - Richard Hughes (Red Hat), - Sarah Jacobus (Microsoft), - Steve McIntyre (Debian), - Teddy Reed, - Thomas Frauendorfer (Miray Software), - Ulrich Bauer (Miray Software), - Yves-Alexis Perez (Debian). We would not be able to succeed without all your hard work. It was very big pleasure to work with you all. Thank you! Daniel [1] https://lists.gnu.org/archive/html/grub-devel/2020-07/msg00034.html [2] https://www.eclypsium.com/2020/07/29/theres-a-hole-in-the-boot/ [3] https://github.com/rhboot/shim/blob/main/SBAT.md [4] Canonical: https://wiki.ubuntu.com/SecurityTeam/KnowledgeBase/GRUB2SecureBootBypass2021 Debian:https://www.debian.org/security/2021-GRUB-UEFI-SecureBoot Red Hat: https://access.redhat.com/security/vulnerabilities/RHSB-2021-003 SUSE: https://www.suse.com/support/kb/doc/?id=19892 [5] https://uefi.org/revocationlistfile [6] https://git.savannah.gnu.org/gitweb/?p=grub.git&view=view+git+repository https://git.savannah.gnu.org/git/grub.git *** CVE-2020-14372 grub2: The acpi command allows privileged user to load crafted ACPI tables when Secure Boot is enabled CWE-184 7.5/CVSS:3.1/AV:L/AC:H/PR:H/UI:N/S:C/C:H/I:H/A:H GRUB2 enables the use of the command acpi even when Secure Boot is signaled by the firmware. An attacker with local root privileges to can drop a small SSDT in /boot/efi and modify grub.cfg to instruct grub to load said SSDT. The SSDT then gets run by the kernel and it overwrites the kernel lock down configuration enabling the attacker to load unsigned kernel modules and kexec unsigned code. Reported-by: Máté Kukri *** CVE-2020-25632 grub2: Use-after-free in rmmod command CWE-416
[SECURITY PATCH 008/117] mmap: Don't register cutmem and badram commands when lockdown is enforced
From: Javier Martinez Canillas
The cutmem and badram commands can be used to remove EFI memory regions
and potentially disable the UEFI Secure Boot. Prevent the commands to be
registered if the GRUB is locked down.
Fixes: CVE-2020-27779
Reported-by: Teddy Reed
Signed-off-by: Javier Martinez Canillas
Reviewed-by: Daniel Kiper
---
docs/grub.texi| 4
grub-core/mmap/mmap.c | 13 +++--
2 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/docs/grub.texi b/docs/grub.texi
index 4ce31c2d4..5dbb02f1c 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -4139,6 +4139,10 @@ this page is to be filtered. This syntax makes it easy
to represent patterns
that are often result of memory damage, due to physical distribution of memory
cells.
+Note: The command is not allowed when lockdown is enforced (@pxref{Lockdown}).
+ This prevents removing EFI memory regions to potentially subvert the
+ security mechanisms provided by the UEFI secure boot.
+
@node blocklist
@subsection blocklist
diff --git a/grub-core/mmap/mmap.c b/grub-core/mmap/mmap.c
index 64684c23d..3cae68364 100644
--- a/grub-core/mmap/mmap.c
+++ b/grub-core/mmap/mmap.c
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -534,12 +535,12 @@ static grub_command_t cmd, cmd_cut;
GRUB_MOD_INIT(mmap)
{
- cmd = grub_register_command ("badram", grub_cmd_badram,
- N_("ADDR1,MASK1[,ADDR2,MASK2[,...]]"),
- N_("Declare memory regions as faulty
(badram)."));
- cmd_cut = grub_register_command ("cutmem", grub_cmd_cutmem,
- N_("FROM[K|M|G] TO[K|M|G]"),
- N_("Remove any memory regions in specified
range."));
+ cmd = grub_register_command_lockdown ("badram", grub_cmd_badram,
+N_("ADDR1,MASK1[,ADDR2,MASK2[,...]]"),
+N_("Declare memory regions as faulty
(badram)."));
+ cmd_cut = grub_register_command_lockdown ("cutmem", grub_cmd_cutmem,
+N_("FROM[K|M|G] TO[K|M|G]"),
+N_("Remove any memory regions in
specified range."));
}
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 016/117] usb: Avoid possible out-of-bound accesses caused by malicious devices
From: Javier Martinez Canillas
The maximum number of configurations and interfaces are fixed but there is
no out-of-bound checking to prevent a malicious USB device to report large
values for these and cause accesses outside the arrays' memory.
Fixes: CVE-2020-25647
Reported-by: Joseph Tartaro
Reported-by: Ilja Van Sprundel
Signed-off-by: Javier Martinez Canillas
Reviewed-by: Daniel Kiper
---
grub-core/bus/usb/usb.c | 15 ---
include/grub/usb.h | 10 +++---
2 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/grub-core/bus/usb/usb.c b/grub-core/bus/usb/usb.c
index 8da5e4c74..7cb3cc230 100644
--- a/grub-core/bus/usb/usb.c
+++ b/grub-core/bus/usb/usb.c
@@ -75,6 +75,9 @@ grub_usb_controller_iterate
(grub_usb_controller_iterate_hook_t hook,
grub_usb_err_t
grub_usb_clear_halt (grub_usb_device_t dev, int endpoint)
{
+ if (endpoint >= GRUB_USB_MAX_TOGGLE)
+return GRUB_USB_ERR_BADDEVICE;
+
dev->toggle[endpoint] = 0;
return grub_usb_control_msg (dev, (GRUB_USB_REQTYPE_OUT
| GRUB_USB_REQTYPE_STANDARD
@@ -134,10 +137,10 @@ grub_usb_device_initialize (grub_usb_device_t dev)
return err;
descdev = &dev->descdev;
- for (i = 0; i < 8; i++)
+ for (i = 0; i < GRUB_USB_MAX_CONF; i++)
dev->config[i].descconf = NULL;
- if (descdev->configcnt == 0)
+ if (descdev->configcnt == 0 || descdev->configcnt > GRUB_USB_MAX_CONF)
{
err = GRUB_USB_ERR_BADDEVICE;
goto fail;
@@ -172,6 +175,12 @@ grub_usb_device_initialize (grub_usb_device_t dev)
/* Skip the configuration descriptor. */
pos = dev->config[i].descconf->length;
+ if (dev->config[i].descconf->numif > GRUB_USB_MAX_IF)
+{
+ err = GRUB_USB_ERR_BADDEVICE;
+ goto fail;
+}
+
/* Read all interfaces. */
for (currif = 0; currif < dev->config[i].descconf->numif; currif++)
{
@@ -217,7 +226,7 @@ grub_usb_device_initialize (grub_usb_device_t dev)
fail:
- for (i = 0; i < 8; i++)
+ for (i = 0; i < GRUB_USB_MAX_CONF; i++)
grub_free (dev->config[i].descconf);
return err;
diff --git a/include/grub/usb.h b/include/grub/usb.h
index 512ae1dd0..6475c552f 100644
--- a/include/grub/usb.h
+++ b/include/grub/usb.h
@@ -23,6 +23,10 @@
#include
#include
+#define GRUB_USB_MAX_CONF8
+#define GRUB_USB_MAX_IF 32
+#define GRUB_USB_MAX_TOGGLE 256
+
typedef struct grub_usb_device *grub_usb_device_t;
typedef struct grub_usb_controller *grub_usb_controller_t;
typedef struct grub_usb_controller_dev *grub_usb_controller_dev_t;
@@ -167,7 +171,7 @@ struct grub_usb_configuration
struct grub_usb_desc_config *descconf;
/* Interfaces associated to this configuration. */
- struct grub_usb_interface interf[32];
+ struct grub_usb_interface interf[GRUB_USB_MAX_IF];
};
struct grub_usb_hub_port
@@ -191,7 +195,7 @@ struct grub_usb_device
struct grub_usb_controller controller;
/* Device configurations (after opening the device). */
- struct grub_usb_configuration config[8];
+ struct grub_usb_configuration config[GRUB_USB_MAX_CONF];
/* Device address. */
int addr;
@@ -203,7 +207,7 @@ struct grub_usb_device
int initialized;
/* Data toggle values (used for bulk transfers only). */
- int toggle[256];
+ int toggle[GRUB_USB_MAX_TOGGLE];
/* Used by libusb wrapper. Schedulded for removal. */
void *data;
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 005/117] efi: Lockdown the GRUB when the UEFI Secure Boot is enabled
From: Javier Martinez Canillas
If the UEFI Secure Boot is enabled then the GRUB must be locked down
to prevent executing code that can potentially be used to subvert its
verification mechanisms.
Signed-off-by: Javier Martinez Canillas
Reviewed-by: Daniel Kiper
---
grub-core/kern/efi/init.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/grub-core/kern/efi/init.c b/grub-core/kern/efi/init.c
index b683bec5a..133346509 100644
--- a/grub-core/kern/efi/init.c
+++ b/grub-core/kern/efi/init.c
@@ -21,6 +21,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -40,8 +41,15 @@ grub_efi_init (void)
/* Initialize the memory management system. */
grub_efi_mm_init ();
- /* Register the shim_lock verifier if UEFI Secure Boot is enabled. */
- grub_shim_lock_verifier_setup ();
+ /*
+ * Lockdown the GRUB and register the shim_lock verifier
+ * if the UEFI Secure Boot is enabled.
+ */
+ if (grub_efi_get_secureboot () == GRUB_EFI_SECUREBOOT_MODE_ENABLED)
+{
+ grub_lockdown ();
+ grub_shim_lock_verifier_setup ();
+}
efi_call_4 (grub_efi_system_table->boot_services->set_watchdog_timer,
0, 0, 0, NULL);
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 029/117] zstd: Initialize seq_t structure fully
From: Darren Kenny
While many compilers will initialize this to zero, not all will, so it
is better to be sure that fields not being explicitly set are at known
values, and there is code that checks this fields value elsewhere in the
code.
Fixes: CID 292440
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
grub-core/lib/zstd/zstd_decompress.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/grub-core/lib/zstd/zstd_decompress.c
b/grub-core/lib/zstd/zstd_decompress.c
index 711b5b6d7..e4b5670c2 100644
--- a/grub-core/lib/zstd/zstd_decompress.c
+++ b/grub-core/lib/zstd/zstd_decompress.c
@@ -1325,7 +1325,7 @@ typedef enum { ZSTD_lo_isRegularOffset,
ZSTD_lo_isLongOffset=1 } ZSTD_longOffset
FORCE_INLINE_TEMPLATE seq_t
ZSTD_decodeSequence(seqState_t* seqState, const ZSTD_longOffset_e longOffsets)
{
-seq_t seq;
+seq_t seq = {0};
U32 const llBits =
seqState->stateLL.table[seqState->stateLL.state].nbAdditionalBits;
U32 const mlBits =
seqState->stateML.table[seqState->stateML.state].nbAdditionalBits;
U32 const ofBits =
seqState->stateOffb.table[seqState->stateOffb.state].nbAdditionalBits;
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 028/117] io/lzopio: Resolve unnecessary self-assignment errors
From: Darren Kenny These 2 assignments are unnecessary since they are just assigning to themselves. Fixes: CID 73643 Signed-off-by: Darren Kenny Reviewed-by: Daniel Kiper --- grub-core/io/lzopio.c | 4 1 file changed, 4 deletions(-) diff --git a/grub-core/io/lzopio.c b/grub-core/io/lzopio.c index 30144857d..a7d442543 100644 --- a/grub-core/io/lzopio.c +++ b/grub-core/io/lzopio.c @@ -125,8 +125,6 @@ read_block_header (struct grub_lzopio *lzopio) sizeof (lzopio->block.ucheck)) != sizeof (lzopio->block.ucheck)) return -1; - - lzopio->block.ucheck = lzopio->block.ucheck; } /* Read checksum of compressed data. */ @@ -143,8 +141,6 @@ read_block_header (struct grub_lzopio *lzopio) sizeof (lzopio->block.ccheck)) != sizeof (lzopio->block.ccheck)) return -1; - - lzopio->block.ccheck = lzopio->block.ccheck; } } -- 2.11.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 020/117] kern/parser: Fix resource leak if argc == 0
From: Darren Kenny
After processing the command-line yet arriving at the point where we are
setting argv, we are allocating memory, even if argc == 0, which makes
no sense since we never put anything into the allocated argv.
The solution is to simply return that we've successfully processed the
arguments but that argc == 0, and also ensure that argv is NULL when
we're not allocating anything in it.
There are only 2 callers of this function, and both are handling a zero
value in argc assuming nothing is allocated in argv.
Fixes: CID 96680
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
grub-core/kern/parser.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/grub-core/kern/parser.c b/grub-core/kern/parser.c
index 619db3122..d1cf061ad 100644
--- a/grub-core/kern/parser.c
+++ b/grub-core/kern/parser.c
@@ -146,6 +146,7 @@ grub_parser_split_cmdline (const char *cmdline,
int i;
*argc = 0;
+ *argv = NULL;
do
{
if (!rd || !*rd)
@@ -207,6 +208,10 @@ grub_parser_split_cmdline (const char *cmdline,
(*argc)++;
}
+ /* If there are no args, then we're done. */
+ if (!*argc)
+return 0;
+
/* Reserve memory for the return values. */
args = grub_malloc (bp - buffer);
if (!args)
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 032/117] disk/ldm: If failed then free vg variable too
From: Paulo Flabiano Smorigo
Fixes: CID 73809
Signed-off-by: Paulo Flabiano Smorigo
Reviewed-by: Daniel Kiper
---
grub-core/disk/ldm.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/grub-core/disk/ldm.c b/grub-core/disk/ldm.c
index 48942549a..c25941ec9 100644
--- a/grub-core/disk/ldm.c
+++ b/grub-core/disk/ldm.c
@@ -199,6 +199,7 @@ make_vg (grub_disk_t disk,
{
grub_free (vg->uuid);
grub_free (vg->name);
+ grub_free (vg);
return NULL;
}
grub_memcpy (vg->uuid, label->group_guid, LDM_GUID_STRLEN);
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 018/117] net/net: Fix possible dereference to of a NULL pointer
From: Darren Kenny
It is always possible that grub_zalloc() could fail, so we should check for
a NULL return. Otherwise we run the risk of dereferencing a NULL pointer.
Fixes: CID 296221
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
grub-core/net/net.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/grub-core/net/net.c b/grub-core/net/net.c
index 04ad10682..4d3eb5c1a 100644
--- a/grub-core/net/net.c
+++ b/grub-core/net/net.c
@@ -86,8 +86,13 @@ grub_net_link_layer_add_address (struct grub_net_card *card,
/* Add sender to cache table. */
if (card->link_layer_table == NULL)
-card->link_layer_table = grub_zalloc (LINK_LAYER_CACHE_SIZE
- * sizeof (card->link_layer_table[0]));
+{
+ card->link_layer_table = grub_zalloc (LINK_LAYER_CACHE_SIZE
+ * sizeof
(card->link_layer_table[0]));
+ if (card->link_layer_table == NULL)
+ return;
+}
+
entry = &(card->link_layer_table[card->new_ll_entry]);
entry->avail = 1;
grub_memcpy (&entry->ll_address, ll, sizeof (entry->ll_address));
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 036/117] zfs: Fix possible negative shift operation
From: Darren Kenny While it is possible for the return value from zfs_log2() to be zero (0), it is quite unlikely, given that the previous assignment to blksz is shifted up by SPA_MINBLOCKSHIFT (9) before 9 is subtracted at the assignment to epbs. But, while unlikely during a normal operation, it may be that a carefully crafted ZFS filesystem could result in a zero (0) value to the dn_datalbkszsec field, which means that the shift left does nothing and assigns zero (0) to blksz, resulting in a negative epbs value. Fixes: CID 73608 Signed-off-by: Darren Kenny Reviewed-by: Daniel Kiper --- grub-core/fs/zfs/zfs.c | 5 + 1 file changed, 5 insertions(+) diff --git a/grub-core/fs/zfs/zfs.c b/grub-core/fs/zfs/zfs.c index b6e1e178d..695e6ea30 100644 --- a/grub-core/fs/zfs/zfs.c +++ b/grub-core/fs/zfs/zfs.c @@ -2670,6 +2670,11 @@ dnode_get (dnode_end_t * mdn, grub_uint64_t objnum, grub_uint8_t type, blksz = grub_zfs_to_cpu16 (mdn->dn.dn_datablkszsec, mdn->endian) << SPA_MINBLOCKSHIFT; epbs = zfs_log2 (blksz) - DNODE_SHIFT; + + /* While this should never happen, we should check that epbs is not negative. */ + if (epbs < 0) +epbs = 0; + blkid = objnum >> epbs; idx = objnum & ((1 << epbs) - 1); -- 2.11.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 007/117] acpi: Don't register the acpi command when locked down
From: Javier Martinez Canillas
The command is not allowed when lockdown is enforced. Otherwise an
attacker can instruct the GRUB to load an SSDT table to overwrite
the kernel lockdown configuration and later load and execute
unsigned code.
Fixes: CVE-2020-14372
Reported-by: Máté Kukri
Signed-off-by: Javier Martinez Canillas
Reviewed-by: Daniel Kiper
---
docs/grub.texi| 5 +
grub-core/commands/acpi.c | 15 ---
2 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/docs/grub.texi b/docs/grub.texi
index c3e015d4f..4ce31c2d4 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -4074,6 +4074,11 @@ Normally, this command will replace the Root System
Description Pointer
(RSDP) in the Extended BIOS Data Area to point to the new tables. If the
@option{--no-ebda} option is used, the new tables will be known only to
GRUB, but may be used by GRUB's EFI emulation.
+
+Note: The command is not allowed when lockdown is enforced (@pxref{Lockdown}).
+ Otherwise an attacker can instruct the GRUB to load an SSDT table to
+ overwrite the kernel lockdown configuration and later load and execute
+ unsigned code.
@end deffn
diff --git a/grub-core/commands/acpi.c b/grub-core/commands/acpi.c
index 5a1499aa0..1215f2a62 100644
--- a/grub-core/commands/acpi.c
+++ b/grub-core/commands/acpi.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#ifdef GRUB_MACHINE_EFI
#include
@@ -775,13 +776,13 @@ static grub_extcmd_t cmd;
GRUB_MOD_INIT(acpi)
{
- cmd = grub_register_extcmd ("acpi", grub_cmd_acpi, 0,
- N_("[-1|-2] [--exclude=TABLE1,TABLE2|"
- "--load-only=TABLE1,TABLE2] FILE1"
- " [FILE2] [...]"),
- N_("Load host ACPI tables and tables "
- "specified by arguments."),
- options);
+ cmd = grub_register_extcmd_lockdown ("acpi", grub_cmd_acpi, 0,
+ N_("[-1|-2] [--exclude=TABLE1,TABLE2|"
+ "--load-only=TABLE1,TABLE2] FILE1"
+ " [FILE2] [...]"),
+ N_("Load host ACPI tables and tables "
+ "specified by arguments."),
+ options);
}
GRUB_MOD_FINI(acpi)
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 024/117] gnulib/regcomp: Fix uninitialized token structure
From: Darren Kenny
The code is assuming that the value of br_token.constraint was
initialized to zero when it wasn't.
While some compilers will ensure that, not all do, so it is better to
fix this explicitly than leave it to chance.
Fixes: CID 73749
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
bootstrap.conf | 2 +-
conf/Makefile.extra-dist| 1 +
grub-core/lib/gnulib-patches/fix-uninit-structure.patch | 11 +++
3 files changed, 13 insertions(+), 1 deletion(-)
create mode 100644 grub-core/lib/gnulib-patches/fix-uninit-structure.patch
diff --git a/bootstrap.conf b/bootstrap.conf
index 4c8c37c16..a7fc34eb3 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -79,7 +79,7 @@ cp -a INSTALL INSTALL.grub
bootstrap_post_import_hook () {
set -e
- for patchname in fix-base64 fix-null-deref fix-unused-value fix-width
no-abort; do
+ for patchname in fix-base64 fix-null-deref fix-uninit-structure
fix-unused-value fix-width no-abort; do
patch -d grub-core/lib/gnulib -p2 \
< "grub-core/lib/gnulib-patches/$patchname.patch"
done
diff --git a/conf/Makefile.extra-dist b/conf/Makefile.extra-dist
index 0d3b74e8e..d83cde427 100644
--- a/conf/Makefile.extra-dist
+++ b/conf/Makefile.extra-dist
@@ -30,6 +30,7 @@ EXTRA_DIST += grub-core/genemuinitheader.sh
EXTRA_DIST += grub-core/lib/gnulib-patches/fix-base64.patch
EXTRA_DIST += grub-core/lib/gnulib-patches/fix-null-deref.patch
+EXTRA_DIST += grub-core/lib/gnulib-patches/fix-uninit-structure.patch
EXTRA_DIST += grub-core/lib/gnulib-patches/fix-unused-value.patch
EXTRA_DIST += grub-core/lib/gnulib-patches/fix-width.patch
EXTRA_DIST += grub-core/lib/gnulib-patches/no-abort.patch
diff --git a/grub-core/lib/gnulib-patches/fix-uninit-structure.patch
b/grub-core/lib/gnulib-patches/fix-uninit-structure.patch
new file mode 100644
index 0..7b4d9f67a
--- /dev/null
+++ b/grub-core/lib/gnulib-patches/fix-uninit-structure.patch
@@ -0,0 +1,11 @@
+--- a/lib/regcomp.c2020-10-22 13:49:06.770168928 +
b/lib/regcomp.c2020-10-22 13:50:37.026528298 +
+@@ -3662,7 +3662,7 @@
+ Idx alloc = 0;
+ #endif /* not RE_ENABLE_I18N */
+ reg_errcode_t ret;
+- re_token_t br_token;
++ re_token_t br_token = {0};
+ bin_tree_t *tree;
+
+ sbcset = (re_bitset_ptr_t) calloc (sizeof (bitset_t), 1);
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 006/117] efi: Use grub_is_lockdown() instead of hardcoding a disabled modules list
From: Javier Martinez Canillas
Now the GRUB can check if it has been locked down and this can be used to
prevent executing commands that can be utilized to circumvent the UEFI
Secure Boot mechanisms. So, instead of hardcoding a list of modules that
have to be disabled, prevent the usage of commands that can be dangerous.
This not only allows the commands to be disabled on other platforms, but
also properly separate the concerns. Since the shim_lock verifier logic
should be only about preventing to run untrusted binaries and not about
defining these kind of policies.
Signed-off-by: Javier Martinez Canillas
Reviewed-by: Daniel Kiper
---
docs/grub.texi | 15 +--
grub-core/commands/i386/wrmsr.c | 5 +++--
grub-core/commands/iorw.c | 19 ++-
grub-core/commands/memrw.c | 19 ++-
grub-core/kern/efi/sb.c | 41 -
5 files changed, 32 insertions(+), 67 deletions(-)
diff --git a/docs/grub.texi b/docs/grub.texi
index 4c980d356..c3e015d4f 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -5405,6 +5405,9 @@ only applies to the particular cpu/core/thread that runs
the command.
Also, if you specify a reserved or unimplemented MSR address, it will
cause a general protection exception (which is not currently being handled)
and the system will reboot.
+
+Note: The command is not allowed when lockdown is enforced (@pxref{Lockdown}).
+ This is done to prevent subverting various security mechanisms.
@end deffn
@node xen_hypervisor
@@ -5915,12 +5918,12 @@ boot and the shim. This functionality is provided by
the shim_lock verifier. It
is built into the @file{core.img} and is registered if the UEFI secure boot is
enabled.
-All modules not stored in the @file{core.img} and the ACPI tables for the
-@command{acpi} command have to be signed, e.g. using PGP. Additionally, the
-@command{iorw}, the @command{memrw} and the @command{wrmsr} commands are
-prohibited if the UEFI secure boot is enabled. This is done due to
-security reasons. All above mentioned requirements are enforced by the
-shim_lock verifier logic.
+All GRUB modules not stored in the @file{core.img}, OS kernels, ACPI tables,
+Device Trees, etc. have to be signed, e.g, using PGP. Additionally, the
commands
+that can be used to subvert the UEFI secure boot mechanism, such as
@command{iorw}
+and @command{memrw} will not be available when the UEFI secure boot is enabled.
+This is done for security reasons and are enforced by the GRUB Lockdown
mechanism
+(@pxref{Lockdown}).
@node Measured Boot
@section Measuring boot components
diff --git a/grub-core/commands/i386/wrmsr.c b/grub-core/commands/i386/wrmsr.c
index fa76f5aed..1b143b888 100644
--- a/grub-core/commands/i386/wrmsr.c
+++ b/grub-core/commands/i386/wrmsr.c
@@ -24,6 +24,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -83,8 +84,8 @@ grub_cmd_msr_write (grub_command_t cmd __attribute__
((unused)), int argc, char
GRUB_MOD_INIT(wrmsr)
{
- cmd_write = grub_register_command ("wrmsr", grub_cmd_msr_write, N_("ADDR
VALUE"),
-N_("Write a value to a CPU model specific
register."));
+ cmd_write = grub_register_command_lockdown ("wrmsr", grub_cmd_msr_write,
N_("ADDR VALUE"),
+ N_("Write a value to a CPU model
specific register."));
}
GRUB_MOD_FINI(wrmsr)
diff --git a/grub-core/commands/iorw.c b/grub-core/commands/iorw.c
index a0c164e54..584baec8f 100644
--- a/grub-core/commands/iorw.c
+++ b/grub-core/commands/iorw.c
@@ -23,6 +23,7 @@
#include
#include
#include
+#include
GRUB_MOD_LICENSE ("GPLv3+");
@@ -131,17 +132,17 @@ GRUB_MOD_INIT(memrw)
N_("PORT"), N_("Read 32-bit value from PORT."),
options);
cmd_write_byte =
-grub_register_command ("outb", grub_cmd_write,
- N_("PORT VALUE [MASK]"),
- N_("Write 8-bit VALUE to PORT."));
+grub_register_command_lockdown ("outb", grub_cmd_write,
+N_("PORT VALUE [MASK]"),
+N_("Write 8-bit VALUE to PORT."));
cmd_write_word =
-grub_register_command ("outw", grub_cmd_write,
- N_("PORT VALUE [MASK]"),
- N_("Write 16-bit VALUE to PORT."));
+grub_register_command_lockdown ("outw", grub_cmd_write,
+N_("PORT VALUE [MASK]"),
+N_("Write 16-bit VALUE to PORT."));
cmd_write_dword =
-grub_register_command ("outl", grub_cmd_write,
- N_("ADDR VALUE [MASK]"),
- N_("Write 32-bit VALUE to PORT."));
+grub_register_command_lockdown ("outl", grub_cmd_write,
+N_("ADDR VALUE [MASK]"),
+
[SECURITY PATCH 043/117] syslinux: Fix memory leak while parsing
From: Darren Kenny
In syslinux_parse_real() the 2 points where return is being called
didn't release the memory stored in buf which is no longer required.
Fixes: CID 176634
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
grub-core/lib/syslinux_parse.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/grub-core/lib/syslinux_parse.c b/grub-core/lib/syslinux_parse.c
index de9fda06f..ff244d2e0 100644
--- a/grub-core/lib/syslinux_parse.c
+++ b/grub-core/lib/syslinux_parse.c
@@ -737,7 +737,10 @@ syslinux_parse_real (struct syslinux_menu *menu)
&& grub_strncasecmp ("help", ptr3, ptr4 - ptr3) == 0))
{
if (helptext (ptr5, file, menu))
- return 1;
+ {
+ grub_free (buf);
+ return 1;
+ }
continue;
}
@@ -757,6 +760,7 @@ syslinux_parse_real (struct syslinux_menu *menu)
}
fail:
grub_file_close (file);
+ grub_free (buf);
return err;
}
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 022/117] kern/efi/mm: Fix possible NULL pointer dereference
From: Darren Kenny
The model of grub_efi_get_memory_map() is that if memory_map is NULL,
then the purpose is to discover how much memory should be allocated to
it for the subsequent call.
The problem here is that with grub_efi_is_finished set to 1, there is no
check at all that the function is being called with a non-NULL memory_map.
While this MAY be true, we shouldn't assume it.
The solution to this is to behave as expected, and if memory_map is NULL,
then don't try to use it and allow memory_map_size to be filled in, and
return 0 as is done later in the code if the buffer is too small (or NULL).
Additionally, drop unneeded ret = 1.
Fixes: CID 96632
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
grub-core/kern/efi/mm.c | 23 ---
1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/grub-core/kern/efi/mm.c b/grub-core/kern/efi/mm.c
index 457772d57..8d380c054 100644
--- a/grub-core/kern/efi/mm.c
+++ b/grub-core/kern/efi/mm.c
@@ -339,16 +339,25 @@ grub_efi_get_memory_map (grub_efi_uintn_t
*memory_map_size,
if (grub_efi_is_finished)
{
int ret = 1;
- if (*memory_map_size < finish_mmap_size)
+
+ if (memory_map != NULL)
{
- grub_memcpy (memory_map, finish_mmap_buf, *memory_map_size);
+ if (*memory_map_size < finish_mmap_size)
+ {
+ grub_memcpy (memory_map, finish_mmap_buf, *memory_map_size);
+ ret = 0;
+ }
+ else
+ grub_memcpy (memory_map, finish_mmap_buf, finish_mmap_size);
+ }
+ else
+ {
+ /*
+ * Incomplete, no buffer to copy into, same as
+ * GRUB_EFI_BUFFER_TOO_SMALL below.
+ */
ret = 0;
}
- else
- {
- grub_memcpy (memory_map, finish_mmap_buf, finish_mmap_size);
- ret = 1;
- }
*memory_map_size = finish_mmap_size;
if (map_key)
*map_key = finish_key;
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 045/117] commands/hashsum: Fix a memory leak
From: Chris Coulson
check_list() uses grub_file_getline(), which allocates a buffer.
If the hash list file contains invalid lines, the function leaks
this buffer when it returns an error.
Fixes: CID 176635
Signed-off-by: Chris Coulson
Reviewed-by: Daniel Kiper
---
grub-core/commands/hashsum.c | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/grub-core/commands/hashsum.c b/grub-core/commands/hashsum.c
index 456ba908b..b8a22b0c8 100644
--- a/grub-core/commands/hashsum.c
+++ b/grub-core/commands/hashsum.c
@@ -128,11 +128,17 @@ check_list (const gcry_md_spec_t *hash, const char
*hashfilename,
high = hextoval (*p++);
low = hextoval (*p++);
if (high < 0 || low < 0)
- return grub_error (GRUB_ERR_BAD_FILE_TYPE, "invalid hash list");
+ {
+ grub_free (buf);
+ return grub_error (GRUB_ERR_BAD_FILE_TYPE, "invalid hash list");
+ }
expected[i] = (high << 4) | low;
}
if ((p[0] != ' ' && p[0] != '\t') || (p[1] != ' ' && p[1] != '\t'))
- return grub_error (GRUB_ERR_BAD_FILE_TYPE, "invalid hash list");
+ {
+ grub_free (buf);
+ return grub_error (GRUB_ERR_BAD_FILE_TYPE, "invalid hash list");
+ }
p += 2;
if (prefix)
{
@@ -140,7 +146,10 @@ check_list (const gcry_md_spec_t *hash, const char
*hashfilename,
filename = grub_xasprintf ("%s/%s", prefix, p);
if (!filename)
- return grub_errno;
+ {
+ grub_free (buf);
+ return grub_errno;
+ }
file = grub_file_open (filename, GRUB_FILE_TYPE_TO_HASH
| (!uncompress ? GRUB_FILE_TYPE_NO_DECOMPRESS
: GRUB_FILE_TYPE_NONE));
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 025/117] gnulib/argp-help: Fix dereference of a possibly NULL state
From: Darren Kenny
All other instances of call to __argp_failure() where there is
a dgettext() call is first checking whether state is NULL before
attempting to dereference it to get the root_argp->argp_domain.
Fixes: CID 292436
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
bootstrap.conf | 2 +-
conf/Makefile.extra-dist| 1 +
grub-core/lib/gnulib-patches/fix-null-state-deref.patch | 12
3 files changed, 14 insertions(+), 1 deletion(-)
create mode 100644 grub-core/lib/gnulib-patches/fix-null-state-deref.patch
diff --git a/bootstrap.conf b/bootstrap.conf
index a7fc34eb3..d71d1b51e 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -79,7 +79,7 @@ cp -a INSTALL INSTALL.grub
bootstrap_post_import_hook () {
set -e
- for patchname in fix-base64 fix-null-deref fix-uninit-structure
fix-unused-value fix-width no-abort; do
+ for patchname in fix-base64 fix-null-deref fix-null-state-deref
fix-uninit-structure fix-unused-value fix-width no-abort; do
patch -d grub-core/lib/gnulib -p2 \
< "grub-core/lib/gnulib-patches/$patchname.patch"
done
diff --git a/conf/Makefile.extra-dist b/conf/Makefile.extra-dist
index d83cde427..a46fc44e7 100644
--- a/conf/Makefile.extra-dist
+++ b/conf/Makefile.extra-dist
@@ -30,6 +30,7 @@ EXTRA_DIST += grub-core/genemuinitheader.sh
EXTRA_DIST += grub-core/lib/gnulib-patches/fix-base64.patch
EXTRA_DIST += grub-core/lib/gnulib-patches/fix-null-deref.patch
+EXTRA_DIST += grub-core/lib/gnulib-patches/fix-null-state-deref.patch
EXTRA_DIST += grub-core/lib/gnulib-patches/fix-uninit-structure.patch
EXTRA_DIST += grub-core/lib/gnulib-patches/fix-unused-value.patch
EXTRA_DIST += grub-core/lib/gnulib-patches/fix-width.patch
diff --git a/grub-core/lib/gnulib-patches/fix-null-state-deref.patch
b/grub-core/lib/gnulib-patches/fix-null-state-deref.patch
new file mode 100644
index 0..813ec09c8
--- /dev/null
+++ b/grub-core/lib/gnulib-patches/fix-null-state-deref.patch
@@ -0,0 +1,12 @@
+--- a/lib/argp-help.c 2020-10-28 14:32:19.189215988 +
b/lib/argp-help.c 2020-10-28 14:38:21.204673940 +
+@@ -145,7 +145,8 @@
+ if (*(int *)((char *)upptr + up->uparams_offs) >= upptr->rmargin)
+ {
+ __argp_failure (state, 0, 0,
+- dgettext (state->root_argp->argp_domain,
++ dgettext (state == NULL ? NULL
++: state->root_argp->argp_domain,
+ "\
+ ARGP_HELP_FMT: %s value is less than or equal to %s"),
+ "rmargin", up->name);
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 009/117] commands: Restrict commands that can load BIOS or DT blobs when locked down
From: Javier Martinez Canillas
There are some more commands that should be restricted when the GRUB is
locked down. Following is the list of commands and reasons to restrict:
* fakebios: creates BIOS-like structures for backward compatibility with
existing OSes. This should not be allowed when locked down.
* loadbios: reads a BIOS dump from storage and loads it. This action
should not be allowed when locked down.
* devicetree: loads a Device Tree blob and passes it to the OS. It replaces
any Device Tree provided by the firmware. This also should
not be allowed when locked down.
Signed-off-by: Javier Martinez Canillas
Reviewed-by: Daniel Kiper
---
docs/grub.texi| 3 +++
grub-core/commands/efi/loadbios.c | 14 +++---
grub-core/loader/arm/linux.c | 6 +++---
grub-core/loader/efi/fdt.c| 4 ++--
4 files changed, 15 insertions(+), 12 deletions(-)
diff --git a/docs/grub.texi b/docs/grub.texi
index 5dbb02f1c..91666781b 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -4324,6 +4324,9 @@ hour, minute, and second unchanged.
Load a device tree blob (.dtb) from a filesystem, for later use by a Linux
kernel. Does not perform merging with any device tree supplied by firmware,
but rather replaces it completely.
+
+Note: The command is not allowed when lockdown is enforced (@pxref{Lockdown}).
+ This is done to prevent subverting various security mechanisms.
@end deffn
@node distrust
diff --git a/grub-core/commands/efi/loadbios.c
b/grub-core/commands/efi/loadbios.c
index d41d521a4..5c7725f8b 100644
--- a/grub-core/commands/efi/loadbios.c
+++ b/grub-core/commands/efi/loadbios.c
@@ -205,14 +205,14 @@ static grub_command_t cmd_fakebios, cmd_loadbios;
GRUB_MOD_INIT(loadbios)
{
- cmd_fakebios = grub_register_command ("fakebios", grub_cmd_fakebios,
- 0, N_("Create BIOS-like structures for"
- " backward compatibility with"
- " existing OS."));
+ cmd_fakebios = grub_register_command_lockdown ("fakebios", grub_cmd_fakebios,
+0, N_("Create BIOS-like
structures for"
+ " backward compatibility
with"
+ " existing OS."));
- cmd_loadbios = grub_register_command ("loadbios", grub_cmd_loadbios,
- N_("BIOS_DUMP [INT10_DUMP]"),
- N_("Load BIOS dump."));
+ cmd_loadbios = grub_register_command_lockdown ("loadbios", grub_cmd_loadbios,
+N_("BIOS_DUMP [INT10_DUMP]"),
+N_("Load BIOS dump."));
}
GRUB_MOD_FINI(loadbios)
diff --git a/grub-core/loader/arm/linux.c b/grub-core/loader/arm/linux.c
index d70c17486..ed23dc71e 100644
--- a/grub-core/loader/arm/linux.c
+++ b/grub-core/loader/arm/linux.c
@@ -493,9 +493,9 @@ GRUB_MOD_INIT (linux)
0, N_("Load Linux."));
cmd_initrd = grub_register_command ("initrd", grub_cmd_initrd,
0, N_("Load initrd."));
- cmd_devicetree = grub_register_command ("devicetree", grub_cmd_devicetree,
- /* TRANSLATORS: DTB stands for device
tree blob. */
- 0, N_("Load DTB file."));
+ cmd_devicetree = grub_register_command_lockdown ("devicetree",
grub_cmd_devicetree,
+ /* TRANSLATORS: DTB stands
for device tree blob. */
+ 0, N_("Load DTB file."));
my_mod = mod;
current_fdt = (const void *) grub_arm_firmware_get_boot_data ();
machine_type = grub_arm_firmware_get_machine_type ();
diff --git a/grub-core/loader/efi/fdt.c b/grub-core/loader/efi/fdt.c
index ee9c5592c..003d07cd8 100644
--- a/grub-core/loader/efi/fdt.c
+++ b/grub-core/loader/efi/fdt.c
@@ -165,8 +165,8 @@ static grub_command_t cmd_devicetree;
GRUB_MOD_INIT (fdt)
{
cmd_devicetree =
-grub_register_command ("devicetree", grub_cmd_devicetree, 0,
- N_("Load DTB file."));
+grub_register_command_lockdown ("devicetree", grub_cmd_devicetree, 0,
+ N_("Load DTB file."));
}
GRUB_MOD_FINI (fdt)
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 048/117] video/fb/fbfill: Fix potential integer overflow
From: Darren Kenny The multiplication of 2 unsigned 32-bit integers may overflow before promotion to unsigned 64-bit. We should ensure that the multiplication is done with overflow detection. Additionally, use grub_sub() for subtraction. Fixes: CID 73640, CID 73697, CID 73702, CID 73823 Signed-off-by: Darren Kenny Signed-off-by: Marco A Benatto Reviewed-by: Daniel Kiper --- grub-core/video/fb/fbfill.c | 17 + 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/grub-core/video/fb/fbfill.c b/grub-core/video/fb/fbfill.c index 11816d07a..a37acd1e2 100644 --- a/grub-core/video/fb/fbfill.c +++ b/grub-core/video/fb/fbfill.c @@ -31,6 +31,7 @@ #include #include #include +#include #include /* Generic filler that works for every supported mode. */ @@ -61,7 +62,9 @@ grub_video_fbfill_direct32 (struct grub_video_fbblit_info *dst, /* Calculate the number of bytes to advance from the end of one line to the beginning of the next line. */ - rowskip = dst->mode_info->pitch - dst->mode_info->bytes_per_pixel * width; + if (grub_mul (dst->mode_info->bytes_per_pixel, width, &rowskip) || + grub_sub (dst->mode_info->pitch, rowskip, &rowskip)) +return; /* Get the start address. */ dstptr = grub_video_fb_get_video_ptr (dst, x, y); @@ -98,7 +101,9 @@ grub_video_fbfill_direct24 (struct grub_video_fbblit_info *dst, #endif /* Calculate the number of bytes to advance from the end of one line to the beginning of the next line. */ - rowskip = dst->mode_info->pitch - dst->mode_info->bytes_per_pixel * width; + if (grub_mul (dst->mode_info->bytes_per_pixel, width, &rowskip) || + grub_sub (dst->mode_info->pitch, rowskip, &rowskip)) +return; /* Get the start address. */ dstptr = grub_video_fb_get_video_ptr (dst, x, y); @@ -131,7 +136,9 @@ grub_video_fbfill_direct16 (struct grub_video_fbblit_info *dst, /* Calculate the number of bytes to advance from the end of one line to the beginning of the next line. */ - rowskip = (dst->mode_info->pitch - dst->mode_info->bytes_per_pixel * width); + if (grub_mul (dst->mode_info->bytes_per_pixel, width, &rowskip) || + grub_sub (dst->mode_info->pitch, rowskip, &rowskip)) +return; /* Get the start address. */ dstptr = grub_video_fb_get_video_ptr (dst, x, y); @@ -161,7 +168,9 @@ grub_video_fbfill_direct8 (struct grub_video_fbblit_info *dst, /* Calculate the number of bytes to advance from the end of one line to the beginning of the next line. */ - rowskip = dst->mode_info->pitch - dst->mode_info->bytes_per_pixel * width; + if (grub_mul (dst->mode_info->bytes_per_pixel, width, &rowskip) || + grub_sub (dst->mode_info->pitch, rowskip, &rowskip)) +return; /* Get the start address. */ dstptr = grub_video_fb_get_video_ptr (dst, x, y); -- 2.11.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 011/117] commands/hdparm: Restrict hdparm command when locked down
From: Javier Martinez Canillas
The command can be used to get/set ATA disk parameters. Some of these can
be dangerous since change the disk behavior. Restrict it when locked down.
Signed-off-by: Javier Martinez Canillas
Reviewed-by: Daniel Kiper
---
grub-core/commands/hdparm.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/grub-core/commands/hdparm.c b/grub-core/commands/hdparm.c
index d3fa9661e..2e2319e64 100644
--- a/grub-core/commands/hdparm.c
+++ b/grub-core/commands/hdparm.c
@@ -436,9 +436,9 @@ static grub_extcmd_t cmd;
GRUB_MOD_INIT(hdparm)
{
- cmd = grub_register_extcmd ("hdparm", grub_cmd_hdparm, 0,
- N_("[OPTIONS] DISK"),
- N_("Get/set ATA disk parameters."), options);
+ cmd = grub_register_extcmd_lockdown ("hdparm", grub_cmd_hdparm, 0,
+ N_("[OPTIONS] DISK"),
+ N_("Get/set ATA disk parameters."),
options);
}
GRUB_MOD_FINI(hdparm)
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 049/117] video/fb/video_fb: Fix multiple integer overflows
From: Darren Kenny
The calculation of the unsigned 64-bit value is being generated by
multiplying 2, signed or unsigned, 32-bit integers which may overflow
before promotion to unsigned 64-bit. Fix all of them.
Fixes: CID 73703, CID 73767, CID 73833
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
grub-core/video/fb/video_fb.c | 52 ++-
1 file changed, 36 insertions(+), 16 deletions(-)
diff --git a/grub-core/video/fb/video_fb.c b/grub-core/video/fb/video_fb.c
index 1a602c8b2..1c9a138dc 100644
--- a/grub-core/video/fb/video_fb.c
+++ b/grub-core/video/fb/video_fb.c
@@ -25,6 +25,7 @@
#include
#include
#include
+#include
GRUB_MOD_LICENSE ("GPLv3+");
@@ -1417,15 +1418,23 @@ doublebuf_blit_update_screen (void)
{
if (framebuffer.current_dirty.first_line
<= framebuffer.current_dirty.last_line)
-grub_memcpy ((char *) framebuffer.pages[0]
-+ framebuffer.current_dirty.first_line
-* framebuffer.back_target->mode_info.pitch,
-(char *) framebuffer.back_target->data
-+ framebuffer.current_dirty.first_line
-* framebuffer.back_target->mode_info.pitch,
-framebuffer.back_target->mode_info.pitch
-* (framebuffer.current_dirty.last_line
- - framebuffer.current_dirty.first_line));
+{
+ grub_size_t copy_size;
+
+ if (grub_sub (framebuffer.current_dirty.last_line,
+ framebuffer.current_dirty.first_line, ©_size) ||
+ grub_mul (framebuffer.back_target->mode_info.pitch, copy_size,
©_size))
+ {
+ /* Shouldn't happen, but if it does we've a bug. */
+ return GRUB_ERR_BUG;
+ }
+
+ grub_memcpy ((char *) framebuffer.pages[0] +
framebuffer.current_dirty.first_line *
+ framebuffer.back_target->mode_info.pitch,
+ (char *) framebuffer.back_target->data +
framebuffer.current_dirty.first_line *
+ framebuffer.back_target->mode_info.pitch,
+ copy_size);
+}
framebuffer.current_dirty.first_line
= framebuffer.back_target->mode_info.height;
framebuffer.current_dirty.last_line = 0;
@@ -1439,7 +1448,7 @@ grub_video_fb_doublebuf_blit_init (struct
grub_video_fbrender_target **back,
volatile void *framebuf)
{
grub_err_t err;
- grub_size_t page_size = mode_info.pitch * mode_info.height;
+ grub_size_t page_size = (grub_size_t) mode_info.pitch * mode_info.height;
framebuffer.offscreen_buffer = grub_zalloc (page_size);
if (! framebuffer.offscreen_buffer)
@@ -1482,12 +1491,23 @@ doublebuf_pageflipping_update_screen (void)
last_line = framebuffer.previous_dirty.last_line;
if (first_line <= last_line)
-grub_memcpy ((char *) framebuffer.pages[framebuffer.render_page]
-+ first_line * framebuffer.back_target->mode_info.pitch,
-(char *) framebuffer.back_target->data
-+ first_line * framebuffer.back_target->mode_info.pitch,
-framebuffer.back_target->mode_info.pitch
-* (last_line - first_line));
+{
+ grub_size_t copy_size;
+
+ if (grub_sub (last_line, first_line, ©_size) ||
+ grub_mul (framebuffer.back_target->mode_info.pitch, copy_size,
©_size))
+ {
+ /* Shouldn't happen, but if it does we've a bug. */
+ return GRUB_ERR_BUG;
+ }
+
+ grub_memcpy ((char *) framebuffer.pages[framebuffer.render_page] +
first_line *
+ framebuffer.back_target->mode_info.pitch,
+ (char *) framebuffer.back_target->data + first_line *
+ framebuffer.back_target->mode_info.pitch,
+ copy_size);
+}
+
framebuffer.previous_dirty = framebuffer.current_dirty;
framebuffer.current_dirty.first_line
= framebuffer.back_target->mode_info.height;
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 015/117] dl: Only allow unloading modules that are not dependencies
From: Javier Martinez Canillas
When a module is attempted to be removed its reference counter is always
decremented. This means that repeated rmmod invocations will cause the
module to be unloaded even if another module depends on it.
This may lead to a use-after-free scenario allowing an attacker to execute
arbitrary code and by-pass the UEFI Secure Boot protection.
While being there, add the extern keyword to some function declarations in
that header file.
Fixes: CVE-2020-25632
Reported-by: Chris Coulson
Signed-off-by: Javier Martinez Canillas
Reviewed-by: Daniel Kiper
---
grub-core/commands/minicmd.c | 7 +--
grub-core/kern/dl.c | 9 +
include/grub/dl.h| 8 +---
3 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/grub-core/commands/minicmd.c b/grub-core/commands/minicmd.c
index 6bbce3128..fa498931e 100644
--- a/grub-core/commands/minicmd.c
+++ b/grub-core/commands/minicmd.c
@@ -140,8 +140,11 @@ grub_mini_cmd_rmmod (struct grub_command *cmd
__attribute__ ((unused)),
if (grub_dl_is_persistent (mod))
return grub_error (GRUB_ERR_BAD_ARGUMENT, "cannot unload persistent
module");
- if (grub_dl_unref (mod) <= 0)
-grub_dl_unload (mod);
+ if (grub_dl_ref_count (mod) > 1)
+return grub_error (GRUB_ERR_BAD_ARGUMENT, "cannot unload referenced
module");
+
+ grub_dl_unref (mod);
+ grub_dl_unload (mod);
return 0;
}
diff --git a/grub-core/kern/dl.c b/grub-core/kern/dl.c
index 48eb5e7b6..48f8a7907 100644
--- a/grub-core/kern/dl.c
+++ b/grub-core/kern/dl.c
@@ -549,6 +549,15 @@ grub_dl_unref (grub_dl_t mod)
return --mod->ref_count;
}
+int
+grub_dl_ref_count (grub_dl_t mod)
+{
+ if (mod == NULL)
+return 0;
+
+ return mod->ref_count;
+}
+
static void
grub_dl_flush_cache (grub_dl_t mod)
{
diff --git a/include/grub/dl.h b/include/grub/dl.h
index f03c03561..b3753c9ca 100644
--- a/include/grub/dl.h
+++ b/include/grub/dl.h
@@ -203,9 +203,11 @@ grub_dl_t EXPORT_FUNC(grub_dl_load) (const char *name);
grub_dl_t grub_dl_load_core (void *addr, grub_size_t size);
grub_dl_t EXPORT_FUNC(grub_dl_load_core_noinit) (void *addr, grub_size_t size);
int EXPORT_FUNC(grub_dl_unload) (grub_dl_t mod);
-void grub_dl_unload_unneeded (void);
-int EXPORT_FUNC(grub_dl_ref) (grub_dl_t mod);
-int EXPORT_FUNC(grub_dl_unref) (grub_dl_t mod);
+extern void grub_dl_unload_unneeded (void);
+extern int EXPORT_FUNC(grub_dl_ref) (grub_dl_t mod);
+extern int EXPORT_FUNC(grub_dl_unref) (grub_dl_t mod);
+extern int EXPORT_FUNC(grub_dl_ref_count) (grub_dl_t mod);
+
extern grub_dl_t EXPORT_VAR(grub_dl_head);
#ifndef GRUB_UTIL
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 026/117] gnulib/regexec: Fix possible null-dereference
From: Darren Kenny
It appears to be possible that the mctx->state_log field may be NULL,
and the name of this function, clean_state_log_if_needed(), suggests
that it should be checking that it is valid to be cleaned before
assuming that it does.
Fixes: CID 86720
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
bootstrap.conf| 3 ++-
conf/Makefile.extra-dist | 1 +
grub-core/lib/gnulib-patches/fix-regexec-null-deref.patch | 12
3 files changed, 15 insertions(+), 1 deletion(-)
create mode 100644 grub-core/lib/gnulib-patches/fix-regexec-null-deref.patch
diff --git a/bootstrap.conf b/bootstrap.conf
index d71d1b51e..22feff5c0 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -79,7 +79,8 @@ cp -a INSTALL INSTALL.grub
bootstrap_post_import_hook () {
set -e
- for patchname in fix-base64 fix-null-deref fix-null-state-deref
fix-uninit-structure fix-unused-value fix-width no-abort; do
+ for patchname in fix-base64 fix-null-deref fix-null-state-deref
fix-regexec-null-deref \
+ fix-uninit-structure fix-unused-value fix-width no-abort; do
patch -d grub-core/lib/gnulib -p2 \
< "grub-core/lib/gnulib-patches/$patchname.patch"
done
diff --git a/conf/Makefile.extra-dist b/conf/Makefile.extra-dist
index a46fc44e7..c0ac65ec3 100644
--- a/conf/Makefile.extra-dist
+++ b/conf/Makefile.extra-dist
@@ -31,6 +31,7 @@ EXTRA_DIST += grub-core/genemuinitheader.sh
EXTRA_DIST += grub-core/lib/gnulib-patches/fix-base64.patch
EXTRA_DIST += grub-core/lib/gnulib-patches/fix-null-deref.patch
EXTRA_DIST += grub-core/lib/gnulib-patches/fix-null-state-deref.patch
+EXTRA_DIST += grub-core/lib/gnulib-patches/fix-regexec-null-deref.patch
EXTRA_DIST += grub-core/lib/gnulib-patches/fix-uninit-structure.patch
EXTRA_DIST += grub-core/lib/gnulib-patches/fix-unused-value.patch
EXTRA_DIST += grub-core/lib/gnulib-patches/fix-width.patch
diff --git a/grub-core/lib/gnulib-patches/fix-regexec-null-deref.patch
b/grub-core/lib/gnulib-patches/fix-regexec-null-deref.patch
new file mode 100644
index 0..db6dac9c9
--- /dev/null
+++ b/grub-core/lib/gnulib-patches/fix-regexec-null-deref.patch
@@ -0,0 +1,12 @@
+--- a/lib/regexec.c2020-10-21 14:25:35.310195912 +
b/lib/regexec.c2020-11-05 10:55:09.621542984 +
+@@ -1692,6 +1692,9 @@
+ {
+ Idx top = mctx->state_log_top;
+
++ if (mctx->state_log == NULL)
++return REG_NOERROR;
++
+ if ((next_state_log_idx >= mctx->input.bufs_len
+&& mctx->input.bufs_len < mctx->input.len)
+ || (next_state_log_idx >= mctx->input.valid_len
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 055/117] loader/xnu: Free driverkey data when an error is detected in grub_xnu_writetree_toheap()
From: Marco A Benatto
... to avoid memory leaks.
Fixes: CID 96640
Signed-off-by: Marco A Benatto
Reviewed-by: Daniel Kiper
---
grub-core/loader/xnu.c | 24
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/grub-core/loader/xnu.c b/grub-core/loader/xnu.c
index eb1446251..1a590dbc0 100644
--- a/grub-core/loader/xnu.c
+++ b/grub-core/loader/xnu.c
@@ -227,26 +227,33 @@ grub_xnu_writetree_toheap (grub_addr_t *target,
grub_size_t *size)
if (! memorymap)
return grub_errno;
- driverkey = (struct grub_xnu_devtree_key *) grub_malloc (sizeof
(*driverkey));
+ driverkey = (struct grub_xnu_devtree_key *) grub_zalloc (sizeof
(*driverkey));
if (! driverkey)
return grub_errno;
driverkey->name = grub_strdup ("DeviceTree");
if (! driverkey->name)
-return grub_errno;
+{
+ err = grub_errno;
+ goto fail;
+}
+
driverkey->datasize = sizeof (*extdesc);
driverkey->next = memorymap->first_child;
memorymap->first_child = driverkey;
driverkey->data = extdesc
= (struct grub_xnu_extdesc *) grub_malloc (sizeof (*extdesc));
if (! driverkey->data)
-return grub_errno;
+{
+ err = grub_errno;
+ goto fail;
+}
/* Allocate the space based on the size with dummy value. */
*size = grub_xnu_writetree_get_size (grub_xnu_devtree_root, "/");
err = grub_xnu_heap_malloc (ALIGN_UP (*size + 1, GRUB_XNU_PAGESIZE),
&src, target);
if (err)
-return err;
+goto fail;
/* Put real data in the dummy. */
extdesc->addr = *target;
@@ -255,6 +262,15 @@ grub_xnu_writetree_toheap (grub_addr_t *target,
grub_size_t *size)
/* Write the tree to heap. */
grub_xnu_writetree_toheap_real (src, grub_xnu_devtree_root, "/");
return GRUB_ERR_NONE;
+
+ fail:
+ memorymap->first_child = NULL;
+
+ grub_free (driverkey->data);
+ grub_free (driverkey->name);
+ grub_free (driverkey);
+
+ return err;
}
/* Find a key or value in parent key. */
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 013/117] loader/xnu: Don't allow loading extension and packages when locked down
From: Javier Martinez Canillas
The shim_lock verifier validates the XNU kernels but no its extensions
and packages. Prevent these to be loaded when the GRUB is locked down.
Signed-off-by: Javier Martinez Canillas
Reviewed-by: Daniel Kiper
---
grub-core/loader/xnu.c | 31 +--
1 file changed, 17 insertions(+), 14 deletions(-)
diff --git a/grub-core/loader/xnu.c b/grub-core/loader/xnu.c
index 9ae4ceb35..44fd5a979 100644
--- a/grub-core/loader/xnu.c
+++ b/grub-core/loader/xnu.c
@@ -1485,20 +1485,23 @@ GRUB_MOD_INIT(xnu)
N_("Load XNU image."));
cmd_kernel64 = grub_register_command ("xnu_kernel64", grub_cmd_xnu_kernel64,
0, N_("Load 64-bit XNU image."));
- cmd_mkext = grub_register_command ("xnu_mkext", grub_cmd_xnu_mkext, 0,
-N_("Load XNU extension package."));
- cmd_kext = grub_register_command ("xnu_kext", grub_cmd_xnu_kext, 0,
- N_("Load XNU extension."));
- cmd_kextdir = grub_register_command ("xnu_kextdir", grub_cmd_xnu_kextdir,
- /* TRANSLATORS: OSBundleRequired is a
- variable name in xnu extensions
- manifests. It behaves mostly like
- GNU/Linux runlevels.
- */
- N_("DIRECTORY [OSBundleRequired]"),
- /* TRANSLATORS: There are many extensions
- in extension directory. */
- N_("Load XNU extension directory."));
+ cmd_mkext = grub_register_command_lockdown ("xnu_mkext", grub_cmd_xnu_mkext,
0,
+ N_("Load XNU extension
package."));
+ cmd_kext = grub_register_command_lockdown ("xnu_kext", grub_cmd_xnu_kext, 0,
+N_("Load XNU extension."));
+ cmd_kextdir = grub_register_command_lockdown ("xnu_kextdir",
grub_cmd_xnu_kextdir,
+ /*
+* TRANSLATORS:
OSBundleRequired is
+* a variable name in xnu
extensions
+* manifests. It behaves mostly
like
+* GNU/Linux runlevels.
+*/
+ N_("DIRECTORY
[OSBundleRequired]"),
+ /*
+* TRANSLATORS: There are many
extensions
+* in extension directory.
+*/
+ N_("Load XNU extension
directory."));
cmd_ramdisk = grub_register_command ("xnu_ramdisk", grub_cmd_xnu_ramdisk, 0,
/* TRANSLATORS: ramdisk here isn't identifier. It can be translated. */
N_("Load XNU ramdisk. "
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 030/117] kern/partition: Check for NULL before dereferencing input string
From: Darren Kenny
There is the possibility that the value of str comes from an external
source and continuing to use it before ever checking its validity is
wrong. So, needs fixing.
Additionally, drop unneeded part initialization.
Fixes: CID 292444
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
grub-core/kern/partition.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/grub-core/kern/partition.c b/grub-core/kern/partition.c
index 2c401b866..3068c4dca 100644
--- a/grub-core/kern/partition.c
+++ b/grub-core/kern/partition.c
@@ -109,11 +109,14 @@ grub_partition_map_probe (const grub_partition_map_t
partmap,
grub_partition_t
grub_partition_probe (struct grub_disk *disk, const char *str)
{
- grub_partition_t part = 0;
+ grub_partition_t part;
grub_partition_t curpart = 0;
grub_partition_t tail;
const char *ptr;
+ if (str == NULL)
+return 0;
+
part = tail = disk->partition;
for (ptr = str; *ptr;)
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 031/117] disk/ldm: Make sure comp data is freed before exiting from make_vg()
From: Marco A Benatto
Several error handling paths in make_vg() do not free comp data before
jumping to fail2 label and returning from the function. This will leak
memory. So, let's fix all issues of that kind.
Fixes: CID 73804
Signed-off-by: Marco A Benatto
Reviewed-by: Daniel Kiper
---
grub-core/disk/ldm.c | 51 ---
1 file changed, 44 insertions(+), 7 deletions(-)
diff --git a/grub-core/disk/ldm.c b/grub-core/disk/ldm.c
index 912e88255..48942549a 100644
--- a/grub-core/disk/ldm.c
+++ b/grub-core/disk/ldm.c
@@ -554,7 +554,11 @@ make_vg (grub_disk_t disk,
comp->segments = grub_calloc (comp->segment_alloc,
sizeof (*comp->segments));
if (!comp->segments)
- goto fail2;
+ {
+ grub_free (comp->internal_id);
+ grub_free (comp);
+ goto fail2;
+ }
}
else
{
@@ -562,7 +566,11 @@ make_vg (grub_disk_t disk,
comp->segment_count = 1;
comp->segments = grub_malloc (sizeof (*comp->segments));
if (!comp->segments)
- goto fail2;
+ {
+ grub_free (comp->internal_id);
+ grub_free (comp);
+ goto fail2;
+ }
comp->segments->start_extent = 0;
comp->segments->extent_count = lv->size;
comp->segments->layout = 0;
@@ -574,15 +582,26 @@ make_vg (grub_disk_t disk,
comp->segments->layout = GRUB_RAID_LAYOUT_SYMMETRIC_MASK;
}
else
- goto fail2;
+ {
+ grub_free (comp->segments);
+ grub_free (comp->internal_id);
+ grub_free (comp);
+ goto fail2;
+ }
ptr += *ptr + 1;
ptr++;
if (!(vblk[i].flags & 0x10))
- goto fail2;
+ {
+ grub_free (comp->segments);
+ grub_free (comp->internal_id);
+ grub_free (comp);
+ goto fail2;
+ }
if (ptr >= vblk[i].dynamic + sizeof (vblk[i].dynamic)
|| ptr + *ptr + 1 >= vblk[i].dynamic
+ sizeof (vblk[i].dynamic))
{
+ grub_free (comp->segments);
grub_free (comp->internal_id);
grub_free (comp);
goto fail2;
@@ -592,6 +611,7 @@ make_vg (grub_disk_t disk,
if (ptr + *ptr + 1 >= vblk[i].dynamic
+ sizeof (vblk[i].dynamic))
{
+ grub_free (comp->segments);
grub_free (comp->internal_id);
grub_free (comp);
goto fail2;
@@ -601,7 +621,12 @@ make_vg (grub_disk_t disk,
comp->segments->nodes = grub_calloc (comp->segments->node_alloc,
sizeof
(*comp->segments->nodes));
if (!lv->segments->nodes)
- goto fail2;
+ {
+ grub_free (comp->segments);
+ grub_free (comp->internal_id);
+ grub_free (comp);
+ goto fail2;
+ }
}
if (lv->segments->node_alloc == lv->segments->node_count)
@@ -611,11 +636,23 @@ make_vg (grub_disk_t disk,
if (grub_mul (lv->segments->node_alloc, 2,
&lv->segments->node_alloc) ||
grub_mul (lv->segments->node_alloc, sizeof
(*lv->segments->nodes), &sz))
- goto fail2;
+ {
+ grub_free (comp->segments->nodes);
+ grub_free (comp->segments);
+ grub_free (comp->internal_id);
+ grub_free (comp);
+ goto fail2;
+ }
t = grub_realloc (lv->segments->nodes, sz);
if (!t)
- goto fail2;
+ {
+ grub_free (comp->segments->nodes);
+ grub_free (comp->segments);
+ grub_free (comp->internal_id);
+ grub_free (comp);
+ goto fail2;
+ }
lv->segments->nodes = t;
}
lv->segments->nodes[lv->segments->node_count].pv = 0;
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 065/117] commands/menuentry: Fix quoting in setparams_prefix()
From: Daniel Axtens Commit 9acdcbf32542 (use single quotes in menuentry setparams command) says that expressing a quoted single quote will require 3 characters. It actually requires (and always did require!) 4 characters: str: a'b => a'\''b len: 3 => 6 (2 for the letters + 4 for the quote) This leads to not allocating enough memory and thus out of bounds writes that have been observed to cause heap corruption. Allocate 4 bytes for each single quote. Commit 22e7dbb2bb81 (Fix quoting in legacy parser.) does the same quoting, but it adds 3 as extra overhead on top of the single byte that the quote already needs. So it's correct. Fixes: 9acdcbf32542 (use single quotes in menuentry setparams command) Fixes: CVE-2021-20233 Reported-by: Daniel Axtens Signed-off-by: Daniel Axtens Reviewed-by: Daniel Kiper --- grub-core/commands/menuentry.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/grub-core/commands/menuentry.c b/grub-core/commands/menuentry.c index 9164df744..720e6d8ea 100644 --- a/grub-core/commands/menuentry.c +++ b/grub-core/commands/menuentry.c @@ -230,7 +230,7 @@ setparams_prefix (int argc, char **args) len += 3; /* 3 = 1 space + 2 quotes */ p = args[i]; while (*p) - len += (*p++ == '\'' ? 3 : 1); + len += (*p++ == '\'' ? 4 : 1); } result = grub_malloc (len + 2); -- 2.11.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 035/117] hfsplus: Check that the volume name length is valid
From: Darren Kenny
HFS+ documentation suggests that the maximum filename and volume name is
255 Unicode characters in length.
So, when converting from big-endian to little-endian, we should ensure
that the name of the volume has a length that is between 0 and 255,
inclusive.
Fixes: CID 73641
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
grub-core/fs/hfsplus.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/grub-core/fs/hfsplus.c b/grub-core/fs/hfsplus.c
index 9c4e4c88c..8fe7c12ed 100644
--- a/grub-core/fs/hfsplus.c
+++ b/grub-core/fs/hfsplus.c
@@ -1012,6 +1012,15 @@ grub_hfsplus_label (grub_device_t device, char **label)
grub_hfsplus_btree_recptr (&data->catalog_tree, node, ptr);
label_len = grub_be_to_cpu16 (catkey->namelen);
+
+ /* Ensure that the length is >= 0. */
+ if (label_len < 0)
+label_len = 0;
+
+ /* Ensure label length is at most 255 Unicode characters. */
+ if (label_len > 255)
+label_len = 255;
+
label_name = grub_calloc (label_len, sizeof (*label_name));
if (!label_name)
{
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 066/117] kern/misc: Always set *end in grub_strtoull()
From: Daniel Axtens
Currently, if there is an error in grub_strtoull(), *end is not set.
This differs from the usual behavior of strtoull(), and also means that
some callers may use an uninitialized value for *end.
Set *end unconditionally.
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/kern/misc.c | 8
1 file changed, 8 insertions(+)
diff --git a/grub-core/kern/misc.c b/grub-core/kern/misc.c
index a278e069b..430e72340 100644
--- a/grub-core/kern/misc.c
+++ b/grub-core/kern/misc.c
@@ -419,6 +419,10 @@ grub_strtoull (const char * restrict str, const char **
const restrict end,
{
grub_error (GRUB_ERR_OUT_OF_RANGE,
N_("overflow is detected"));
+
+ if (end)
+*end = (char *) str;
+
return ~0ULL;
}
@@ -430,6 +434,10 @@ grub_strtoull (const char * restrict str, const char **
const restrict end,
{
grub_error (GRUB_ERR_BAD_NUMBER,
N_("unrecognized number"));
+
+ if (end)
+*end = (char *) str;
+
return 0;
}
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 041/117] libgcrypt/mpi: Fix possible unintended sign extension
From: Darren Kenny The array of unsigned char gets promoted to a signed 32-bit int before it is finally promoted to a size_t. There is the possibility that this may result in the signed-bit being set for the intermediate signed 32-bit int. We should ensure that the promotion is to the correct type before we bitwise-OR the values. Fixes: CID 96697 Signed-off-by: Darren Kenny Reviewed-by: Daniel Kiper --- grub-core/lib/libgcrypt/mpi/mpicoder.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/grub-core/lib/libgcrypt/mpi/mpicoder.c b/grub-core/lib/libgcrypt/mpi/mpicoder.c index a3435ed14..7ecad27b2 100644 --- a/grub-core/lib/libgcrypt/mpi/mpicoder.c +++ b/grub-core/lib/libgcrypt/mpi/mpicoder.c @@ -458,7 +458,7 @@ gcry_mpi_scan (struct gcry_mpi **ret_mpi, enum gcry_mpi_format format, if (len && len < 4) return gcry_error (GPG_ERR_TOO_SHORT); - n = (s[0] << 24 | s[1] << 16 | s[2] << 8 | s[3]); + n = ((size_t)s[0] << 24 | (size_t)s[1] << 16 | (size_t)s[2] << 8 | (size_t)s[3]); s += 4; if (len) len -= 4; -- 2.11.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 017/117] mmap: Fix memory leak when iterating over mapped memory
From: Darren Kenny
When returning from grub_mmap_iterate() the memory allocated to present
is not being released causing it to leak.
Fixes: CID 96655
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
grub-core/mmap/mmap.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/grub-core/mmap/mmap.c b/grub-core/mmap/mmap.c
index 3cae68364..c8c8312c5 100644
--- a/grub-core/mmap/mmap.c
+++ b/grub-core/mmap/mmap.c
@@ -270,6 +270,7 @@ grub_mmap_iterate (grub_memory_hook_t hook, void *hook_data)
hook_data))
{
grub_free (ctx.scanline_events);
+ grub_free (present);
return GRUB_ERR_NONE;
}
@@ -282,6 +283,7 @@ grub_mmap_iterate (grub_memory_hook_t hook, void *hook_data)
}
grub_free (ctx.scanline_events);
+ grub_free (present);
return GRUB_ERR_NONE;
}
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 072/117] fs/hfsplus: Don't fetch a key beyond the end of the node
From: Daniel Axtens Otherwise you get a wild pointer, leading to a bunch of invalid reads. Check it falls inside the given node. Signed-off-by: Daniel Axtens Reviewed-by: Daniel Kiper --- grub-core/fs/hfsplus.c | 4 1 file changed, 4 insertions(+) diff --git a/grub-core/fs/hfsplus.c b/grub-core/fs/hfsplus.c index 8fe7c12ed..1c7791b02 100644 --- a/grub-core/fs/hfsplus.c +++ b/grub-core/fs/hfsplus.c @@ -635,6 +635,10 @@ grub_hfsplus_btree_search (struct grub_hfsplus_btree *btree, pointer = ((char *) currkey + grub_be_to_cpu16 (currkey->keylen) + 2); + + if ((char *) pointer > node + btree->nodesize - 2) + return grub_error (GRUB_ERR_BAD_FS, "HFS+ key beyond end of node"); + currnode = grub_be_to_cpu32 (grub_get_unaligned32 (pointer)); match = 1; } -- 2.11.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 051/117] video/readers/jpeg: Test for an invalid next marker reference from a jpeg file
From: Darren Kenny
While it may never happen, and potentially could be caught at the end of
the function, it is worth checking up front for a bad reference to the
next marker just in case of a maliciously crafted file being provided.
Fixes: CID 73694
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
grub-core/video/readers/jpeg.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/grub-core/video/readers/jpeg.c b/grub-core/video/readers/jpeg.c
index 31359a4c9..0b6ce3cee 100644
--- a/grub-core/video/readers/jpeg.c
+++ b/grub-core/video/readers/jpeg.c
@@ -253,6 +253,12 @@ grub_jpeg_decode_quan_table (struct grub_jpeg_data *data)
next_marker = data->file->offset;
next_marker += grub_jpeg_get_word (data);
+ if (next_marker > data->file->size)
+{
+ /* Should never be set beyond the size of the file. */
+ return grub_error (GRUB_ERR_BAD_FILE_TYPE, "jpeg: invalid next
reference");
+}
+
while (data->file->offset + sizeof (data->quan_table[id]) + 1
<= next_marker)
{
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 073/117] fs/hfsplus: Don't use uninitialized data on corrupt filesystems
From: Daniel Axtens
Valgrind identified the following use of uninitialized data:
==2782220== Conditional jump or move depends on uninitialised value(s)
==2782220==at 0x42B364: grub_hfsplus_btree_search (hfsplus.c:566)
==2782220==by 0x42B21D: grub_hfsplus_read_block (hfsplus.c:185)
==2782220==by 0x42A693: grub_fshelp_read_file (fshelp.c:386)
==2782220==by 0x42C598: grub_hfsplus_read_file (hfsplus.c:219)
==2782220==by 0x42C598: grub_hfsplus_mount (hfsplus.c:330)
==2782220==by 0x42B8C5: grub_hfsplus_dir (hfsplus.c:958)
==2782220==by 0x4C1AE6: grub_fs_probe (fs.c:73)
==2782220==by 0x407C94: grub_ls_list_files (ls.c:186)
==2782220==by 0x407C94: grub_cmd_ls (ls.c:284)
==2782220==by 0x4D7130: grub_extcmd_dispatcher (extcmd.c:55)
==2782220==by 0x4045A6: execute_command (grub-fstest.c:59)
==2782220==by 0x4045A6: fstest (grub-fstest.c:433)
==2782220==by 0x4045A6: main (grub-fstest.c:772)
==2782220== Uninitialised value was created by a heap allocation
==2782220==at 0x483C7F3: malloc (in
/usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==2782220==by 0x4C0305: grub_malloc (mm.c:42)
==2782220==by 0x42C21D: grub_hfsplus_mount (hfsplus.c:239)
==2782220==by 0x42B8C5: grub_hfsplus_dir (hfsplus.c:958)
==2782220==by 0x4C1AE6: grub_fs_probe (fs.c:73)
==2782220==by 0x407C94: grub_ls_list_files (ls.c:186)
==2782220==by 0x407C94: grub_cmd_ls (ls.c:284)
==2782220==by 0x4D7130: grub_extcmd_dispatcher (extcmd.c:55)
==2782220==by 0x4045A6: execute_command (grub-fstest.c:59)
==2782220==by 0x4045A6: fstest (grub-fstest.c:433)
==2782220==by 0x4045A6: main (grub-fstest.c:772)
This happens when the process of reading the catalog file goes sufficiently
wrong that there's an attempt to read the extent overflow file, which has
not yet been loaded. Keep track of when the extent overflow file is
fully loaded and refuse to use it before then.
The load valgrind doesn't like is btree->nodesize, and that's then used
to allocate a data structure. It looks like there are subsequently a lot
of reads based on that pointer so OOB reads are likely, and indeed crashes
(albeit difficult-to-replicate ones) have been observed in fuzzing.
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/fs/hfsplus.c | 14 ++
include/grub/hfsplus.h | 2 ++
2 files changed, 16 insertions(+)
diff --git a/grub-core/fs/hfsplus.c b/grub-core/fs/hfsplus.c
index 1c7791b02..361e5be49 100644
--- a/grub-core/fs/hfsplus.c
+++ b/grub-core/fs/hfsplus.c
@@ -177,6 +177,17 @@ grub_hfsplus_read_block (grub_fshelp_node_t node,
grub_disk_addr_t fileblock)
break;
}
+ /*
+ * If the extent overflow tree isn't ready yet, we can't look
+ * in it. This can happen where the catalog file is corrupted.
+ */
+ if (!node->data->extoverflow_tree_ready)
+ {
+ grub_error (GRUB_ERR_BAD_FS,
+ "attempted to read extent overflow tree before loading");
+ break;
+ }
+
/* Set up the key to look for in the extent overflow file. */
extoverflow.extkey.fileid = node->fileid;
extoverflow.extkey.type = 0;
@@ -241,6 +252,7 @@ grub_hfsplus_mount (grub_disk_t disk)
return 0;
data->disk = disk;
+ data->extoverflow_tree_ready = 0;
/* Read the bootblock. */
grub_disk_read (disk, GRUB_HFSPLUS_SBLOCK, 0, sizeof (volheader),
@@ -357,6 +369,8 @@ grub_hfsplus_mount (grub_disk_t disk)
if (data->extoverflow_tree.nodesize < 2)
goto fail;
+ data->extoverflow_tree_ready = 1;
+
if (grub_hfsplus_read_file (&data->attr_tree.file, 0, 0,
sizeof (struct grub_hfsplus_btnode),
sizeof (header), (char *) &header) <= 0)
diff --git a/include/grub/hfsplus.h b/include/grub/hfsplus.h
index 117740ae2..e14dd31ff 100644
--- a/include/grub/hfsplus.h
+++ b/include/grub/hfsplus.h
@@ -113,6 +113,8 @@ struct grub_hfsplus_data
struct grub_hfsplus_btree extoverflow_tree;
struct grub_hfsplus_btree attr_tree;
+ int extoverflow_tree_ready;
+
struct grub_hfsplus_file dirroot;
struct grub_hfsplus_file opened_file;
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 052/117] gfxmenu/gui_list: Remove code that coverity is flagging as dead
From: Darren Kenny
The test of value for NULL before calling grub_strdup() is not required,
since the if condition prior to this has already tested for value being
NULL and cannot reach this code if it is.
Fixes: CID 73659
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
grub-core/gfxmenu/gui_list.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/grub-core/gfxmenu/gui_list.c b/grub-core/gfxmenu/gui_list.c
index 01477cdf2..df334a6c5 100644
--- a/grub-core/gfxmenu/gui_list.c
+++ b/grub-core/gfxmenu/gui_list.c
@@ -771,7 +771,7 @@ list_set_property (void *vself, const char *name, const
char *value)
{
self->need_to_recreate_boxes = 1;
grub_free (self->selected_item_box_pattern);
- self->selected_item_box_pattern = value ? grub_strdup (value) : 0;
+ self->selected_item_box_pattern = grub_strdup (value);
self->selected_item_box_pattern_inherit = 0;
}
}
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 083/117] io/gzio: Add init_dynamic_block() clean up if unpacking codes fails
From: Daniel Axtens
init_dynamic_block() didn't clean up gzio->tl and td in some error
paths. This left td pointing to part of tl. Then in grub_gzio_close(),
when tl was freed the storage for td would also be freed. The code then
attempts to free td explicitly, performing a UAF and then a double free.
Explicitly clean up tl and td in the error paths.
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/io/gzio.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/grub-core/io/gzio.c b/grub-core/io/gzio.c
index 4a8eaeae2..4236f0fd4 100644
--- a/grub-core/io/gzio.c
+++ b/grub-core/io/gzio.c
@@ -953,7 +953,7 @@ init_dynamic_block (grub_gzio_t gzio)
if ((unsigned) i + j > n)
{
grub_error (GRUB_ERR_BAD_COMPRESSED_DATA, "too many codes found");
- return;
+ goto fail;
}
while (j--)
ll[i++] = l;
@@ -966,7 +966,7 @@ init_dynamic_block (grub_gzio_t gzio)
if ((unsigned) i + j > n)
{
grub_error (GRUB_ERR_BAD_COMPRESSED_DATA, "too many codes found");
- return;
+ goto fail;
}
while (j--)
ll[i++] = 0;
@@ -981,7 +981,7 @@ init_dynamic_block (grub_gzio_t gzio)
if ((unsigned) i + j > n)
{
grub_error (GRUB_ERR_BAD_COMPRESSED_DATA, "too many codes found");
- return;
+ goto fail;
}
while (j--)
ll[i++] = 0;
@@ -1019,6 +1019,12 @@ init_dynamic_block (grub_gzio_t gzio)
/* indicate we're now working on a block */
gzio->code_state = 0;
gzio->block_len++;
+ return;
+
+ fail:
+ huft_free (gzio->tl);
+ gzio->td = NULL;
+ gzio->tl = NULL;
}
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 023/117] gnulib/regexec: Resolve unused variable
From: Darren Kenny
This is a really minor issue where a variable is being assigned to but
not checked before it is overwritten again.
The reason for this issue is that we are not building with DEBUG set and
this in turn means that the assert() that reads the value of the
variable match_last is being processed out.
The solution, move the assignment to match_last in to an ifdef DEBUG too.
Fixes: CID 292459
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
bootstrap.conf | 2 +-
conf/Makefile.extra-dist| 1 +
grub-core/lib/gnulib-patches/fix-unused-value.patch | 14 ++
3 files changed, 16 insertions(+), 1 deletion(-)
create mode 100644 grub-core/lib/gnulib-patches/fix-unused-value.patch
diff --git a/bootstrap.conf b/bootstrap.conf
index 6d383240e..4c8c37c16 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -79,7 +79,7 @@ cp -a INSTALL INSTALL.grub
bootstrap_post_import_hook () {
set -e
- for patchname in fix-base64 fix-null-deref fix-width no-abort; do
+ for patchname in fix-base64 fix-null-deref fix-unused-value fix-width
no-abort; do
patch -d grub-core/lib/gnulib -p2 \
< "grub-core/lib/gnulib-patches/$patchname.patch"
done
diff --git a/conf/Makefile.extra-dist b/conf/Makefile.extra-dist
index 37dc3aa10..0d3b74e8e 100644
--- a/conf/Makefile.extra-dist
+++ b/conf/Makefile.extra-dist
@@ -30,6 +30,7 @@ EXTRA_DIST += grub-core/genemuinitheader.sh
EXTRA_DIST += grub-core/lib/gnulib-patches/fix-base64.patch
EXTRA_DIST += grub-core/lib/gnulib-patches/fix-null-deref.patch
+EXTRA_DIST += grub-core/lib/gnulib-patches/fix-unused-value.patch
EXTRA_DIST += grub-core/lib/gnulib-patches/fix-width.patch
EXTRA_DIST += grub-core/lib/gnulib-patches/no-abort.patch
diff --git a/grub-core/lib/gnulib-patches/fix-unused-value.patch
b/grub-core/lib/gnulib-patches/fix-unused-value.patch
new file mode 100644
index 0..ba51f1bf2
--- /dev/null
+++ b/grub-core/lib/gnulib-patches/fix-unused-value.patch
@@ -0,0 +1,14 @@
+--- a/lib/regexec.c2020-10-21 14:25:35.310195912 +
b/lib/regexec.c2020-10-21 14:32:07.961765604 +
+@@ -828,7 +828,11 @@
+ break;
+ if (__glibc_unlikely (err != REG_NOMATCH))
+ goto free_return;
++#ifdef DEBUG
++/* Only used for assertion below when DEBUG is set, otherwise
++ it will be over-written when we loop around. */
+ match_last = -1;
++#endif
+ }
+ else
+ break; /* We found a match. */
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 063/117] lib/arg: Block repeated short options that require an argument
From: Daniel Axtens
Fuzzing found the following crash:
search -hf
We didn't allocate enough option space for 13 hints because the
allocation code counts the number of discrete arguments (i.e. argc).
However, the shortopt parsing code will happily keep processing
a combination of short options without checking if those short
options require an argument. This means you can easily end writing
past the allocated option space.
This fixes a OOB write which can cause heap corruption.
Fixes: CVE-2021-20225
Reported-by: Daniel Axtens
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/lib/arg.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/grub-core/lib/arg.c b/grub-core/lib/arg.c
index 8439a0062..ade82d5dc 100644
--- a/grub-core/lib/arg.c
+++ b/grub-core/lib/arg.c
@@ -299,6 +299,19 @@ grub_arg_parse (grub_extcmd_t cmd, int argc, char **argv,
it can have an argument value. */
if (*curshort)
{
+ /*
+ * Only permit further short opts if this one doesn't
+ * require a value.
+ */
+ if (opt->type != ARG_TYPE_NONE &&
+ !(opt->flags & GRUB_ARG_OPTION_OPTIONAL))
+ {
+ grub_error (GRUB_ERR_BAD_ARGUMENT,
+ N_("missing mandatory option for `%s'"),
+ opt->longarg);
+ goto fail;
+ }
+
if (parse_option (cmd, opt, 0, usr) || grub_errno)
goto fail;
}
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 079/117] fs/nilfs2: Reject too-large keys
From: Daniel Axtens
NILFS2 has up to 7 keys, per the data structure. Do not permit array
indices in excess of that.
This catches some OOB reads. I don't know how controllable the invalidly
read data is or if that could be used later in the program.
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/fs/nilfs2.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/grub-core/fs/nilfs2.c b/grub-core/fs/nilfs2.c
index 082326f38..20053caf5 100644
--- a/grub-core/fs/nilfs2.c
+++ b/grub-core/fs/nilfs2.c
@@ -569,6 +569,11 @@ grub_nilfs2_btree_lookup (struct grub_nilfs2_data *data,
static inline grub_uint64_t
grub_nilfs2_direct_lookup (struct grub_nilfs2_inode *inode, grub_uint64_t key)
{
+ if (1 + key > 6)
+{
+ grub_error (GRUB_ERR_BAD_FS, "key is too large");
+ return 0x;
+}
return grub_le_to_cpu64 (inode->i_bmap[1 + key]);
}
@@ -584,7 +589,7 @@ grub_nilfs2_bmap_lookup (struct grub_nilfs2_data *data,
{
grub_uint64_t ptr;
ptr = grub_nilfs2_direct_lookup (inode, key);
- if (need_translate)
+ if (ptr != ((grub_uint64_t) 0x) && need_translate)
ptr = grub_nilfs2_dat_translate (data, ptr);
return ptr;
}
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 027/117] gnulib/regcomp: Fix uninitialized re_token
From: Darren Kenny
This issue has been fixed in the latest version of gnulib, so to
maintain consistency, I've backported that change rather than doing
something different.
Fixes: CID 73828
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
bootstrap.conf| 4 ++--
conf/Makefile.extra-dist | 1 +
.../lib/gnulib-patches/fix-regcomp-uninit-token.patch | 15 +++
3 files changed, 18 insertions(+), 2 deletions(-)
create mode 100644 grub-core/lib/gnulib-patches/fix-regcomp-uninit-token.patch
diff --git a/bootstrap.conf b/bootstrap.conf
index 22feff5c0..6b043fc35 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -79,8 +79,8 @@ cp -a INSTALL INSTALL.grub
bootstrap_post_import_hook () {
set -e
- for patchname in fix-base64 fix-null-deref fix-null-state-deref
fix-regexec-null-deref \
- fix-uninit-structure fix-unused-value fix-width no-abort; do
+ for patchname in fix-base64 fix-null-deref fix-null-state-deref
fix-regcomp-uninit-token \
+ fix-regexec-null-deref fix-uninit-structure fix-unused-value fix-width
no-abort; do
patch -d grub-core/lib/gnulib -p2 \
< "grub-core/lib/gnulib-patches/$patchname.patch"
done
diff --git a/conf/Makefile.extra-dist b/conf/Makefile.extra-dist
index c0ac65ec3..8f1485d52 100644
--- a/conf/Makefile.extra-dist
+++ b/conf/Makefile.extra-dist
@@ -31,6 +31,7 @@ EXTRA_DIST += grub-core/genemuinitheader.sh
EXTRA_DIST += grub-core/lib/gnulib-patches/fix-base64.patch
EXTRA_DIST += grub-core/lib/gnulib-patches/fix-null-deref.patch
EXTRA_DIST += grub-core/lib/gnulib-patches/fix-null-state-deref.patch
+EXTRA_DIST += grub-core/lib/gnulib-patches/fix-regcomp-uninit-token.patch
EXTRA_DIST += grub-core/lib/gnulib-patches/fix-regexec-null-deref.patch
EXTRA_DIST += grub-core/lib/gnulib-patches/fix-uninit-structure.patch
EXTRA_DIST += grub-core/lib/gnulib-patches/fix-unused-value.patch
diff --git a/grub-core/lib/gnulib-patches/fix-regcomp-uninit-token.patch
b/grub-core/lib/gnulib-patches/fix-regcomp-uninit-token.patch
new file mode 100644
index 0..02e06315d
--- /dev/null
+++ b/grub-core/lib/gnulib-patches/fix-regcomp-uninit-token.patch
@@ -0,0 +1,15 @@
+--- a/lib/regcomp.c2020-11-24 17:06:08.159223858 +
b/lib/regcomp.c2020-11-24 17:06:15.630253923 +
+@@ -3808,11 +3808,7 @@
+ create_tree (re_dfa_t *dfa, bin_tree_t *left, bin_tree_t *right,
+re_token_type_t type)
+ {
+- re_token_t t;
+-#if defined GCC_LINT || defined lint
+- memset (&t, 0, sizeof t);
+-#endif
+- t.type = type;
++ re_token_t t = { .type = type };
+ return create_token_tree (dfa, left, right, &t);
+ }
+
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 070/117] term/gfxterm: Don't set up a font with glyphs that are too big
From: Daniel Axtens Catch the case where we have a font so big that it causes the number of rows or columns to be 0. Currently we continue and allocate a virtual_screen.text_buffer of size 0. We then try to use that for glpyhs and things go badly. On the emu platform, malloc() may give us a valid pointer, in which case we'll access heap memory which we shouldn't. Alternatively, it may give us NULL, in which case we'll crash. For other platforms, if I understand grub_memalign() correctly, we will receive a valid but small allocation that we will very likely later overrun. Prevent the creation of a virtual screen that isn't at least 40 cols by 12 rows. This is arbitrary, but it seems that if your width or height is half a standard 80x24 terminal, you're probably going to struggle to read anything anyway. Signed-off-by: Daniel Axtens Reviewed-by: Daniel Kiper --- grub-core/term/gfxterm.c | 9 + 1 file changed, 9 insertions(+) diff --git a/grub-core/term/gfxterm.c b/grub-core/term/gfxterm.c index af7c090a3..b40fcce91 100644 --- a/grub-core/term/gfxterm.c +++ b/grub-core/term/gfxterm.c @@ -232,6 +232,15 @@ grub_virtual_screen_setup (unsigned int x, unsigned int y, virtual_screen.columns = virtual_screen.width / virtual_screen.normal_char_width; virtual_screen.rows = virtual_screen.height / virtual_screen.normal_char_height; + /* + * There must be a minimum number of rows and columns for the screen to + * make sense. Arbitrarily pick half of 80x24. If either dimensions is 0 + * we would allocate 0 bytes for the text_buffer. + */ + if (virtual_screen.columns < 40 || virtual_screen.rows < 12) +return grub_error (GRUB_ERR_BAD_FONT, + "font: glyphs too large to fit on screen"); + /* Allocate memory for text buffer. */ virtual_screen.text_buffer = (struct grub_colored_char *) grub_malloc (virtual_screen.columns -- 2.11.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 087/117] disk/lvm: Don't blast past the end of the circular metadata buffer
From: Daniel Axtens
This catches at least some OOB reads, and it's possible I suppose that
if 2 * mda_size is less than GRUB_LVM_MDA_HEADER_SIZE it might catch some
OOB writes too (although that hasn't showed up as a crash in fuzzing yet).
It's a bit ugly and I'd appreciate better suggestions.
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/disk/lvm.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/grub-core/disk/lvm.c b/grub-core/disk/lvm.c
index 6988fe492..b70d7d79f 100644
--- a/grub-core/disk/lvm.c
+++ b/grub-core/disk/lvm.c
@@ -251,6 +251,16 @@ grub_lvm_detect (grub_disk_t disk,
if (grub_le_to_cpu64 (rlocn->offset) + grub_le_to_cpu64 (rlocn->size) >
grub_le_to_cpu64 (mdah->size))
{
+ if (2 * mda_size < GRUB_LVM_MDA_HEADER_SIZE ||
+ (grub_le_to_cpu64 (rlocn->offset) + grub_le_to_cpu64 (rlocn->size) -
+ grub_le_to_cpu64 (mdah->size) > mda_size - GRUB_LVM_MDA_HEADER_SIZE))
+ {
+#ifdef GRUB_UTIL
+ grub_util_info ("cannot copy metadata wrap in circular buffer");
+#endif
+ goto fail2;
+ }
+
/* Metadata is circular. Copy the wrap in place. */
grub_memcpy (metadatabuf + mda_size,
metadatabuf + GRUB_LVM_MDA_HEADER_SIZE,
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 064/117] script/execute: Don't crash on a "for" loop with no items
From: Daniel Axtens
The following crashes the parser:
for x in; do
0
done
This is because grub_script_arglist_to_argv() doesn't consider the
possibility that arglist is NULL. Catch that explicitly.
This avoids a NULL pointer dereference.
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/script/execute.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/grub-core/script/execute.c b/grub-core/script/execute.c
index b88765f1d..25158407d 100644
--- a/grub-core/script/execute.c
+++ b/grub-core/script/execute.c
@@ -624,6 +624,9 @@ grub_script_arglist_to_argv (struct grub_script_arglist
*arglist,
struct grub_script_arg *arg = 0;
struct grub_script_argv result = { 0, 0, 0 };
+ if (arglist == NULL)
+return 1;
+
for (; arglist && arglist->arg; arglist = arglist->next)
{
if (grub_script_argv_next (&result))
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 033/117] disk/ldm: Fix memory leak on uninserted lv references
From: Darren Kenny
The problem here is that the memory allocated to the variable lv is not
yet inserted into the list that is being processed at the label fail2.
As we can already see at line 342, which correctly frees lv before going
to fail2, we should also be doing that at these earlier jumps to fail2.
Fixes: CID 73824
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
grub-core/disk/ldm.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/grub-core/disk/ldm.c b/grub-core/disk/ldm.c
index c25941ec9..4577a51dc 100644
--- a/grub-core/disk/ldm.c
+++ b/grub-core/disk/ldm.c
@@ -321,7 +321,10 @@ make_vg (grub_disk_t disk,
lv->visible = 1;
lv->segments = grub_zalloc (sizeof (*lv->segments));
if (!lv->segments)
- goto fail2;
+ {
+ grub_free (lv);
+ goto fail2;
+ }
lv->segments->start_extent = 0;
lv->segments->type = GRUB_DISKFILTER_MIRROR;
lv->segments->node_count = 0;
@@ -329,7 +332,10 @@ make_vg (grub_disk_t disk,
lv->segments->nodes = grub_calloc (lv->segments->node_alloc,
sizeof (*lv->segments->nodes));
if (!lv->segments->nodes)
- goto fail2;
+ {
+ grub_free (lv);
+ goto fail2;
+ }
ptr = vblk[i].dynamic;
if (ptr + *ptr + 1 >= vblk[i].dynamic
+ sizeof (vblk[i].dynamic))
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 076/117] fs/jfs: Do not move to leaf level if name length is negative
From: Daniel Axtens
Fuzzing JFS revealed crashes where a negative number would be passed
to le_to_cpu16_copy(). There it would be cast to a large positive number
and the copy would read and write off the end of the respective buffers.
Catch this at the top as well as the bottom of the loop.
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/fs/jfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/grub-core/fs/jfs.c b/grub-core/fs/jfs.c
index d5a6d6527..e5bbda61c 100644
--- a/grub-core/fs/jfs.c
+++ b/grub-core/fs/jfs.c
@@ -567,7 +567,7 @@ grub_jfs_getent (struct grub_jfs_diropen *diro)
/* Move down to the leaf level. */
nextent = leaf->next;
- if (leaf->next != 255)
+ if (leaf->next != 255 && len > 0)
do
{
next_leaf = &diro->next_leaf[nextent];
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 069/117] video/readers/jpeg: Don't decode data before start of stream
From: Daniel Axtens When a start of stream marker is encountered, we call grub_jpeg_decode_sos() which allocates space for a bitmap. When a restart marker is encountered, we call grub_jpeg_decode_data() which then fills in that bitmap. If we get a restart marker before the start of stream marker, we will attempt to write to a bitmap_ptr that hasn't been allocated. Catch this and bail out. This fixes an attempt to write to NULL. Signed-off-by: Daniel Axtens Reviewed-by: Daniel Kiper --- grub-core/video/readers/jpeg.c | 4 1 file changed, 4 insertions(+) diff --git a/grub-core/video/readers/jpeg.c b/grub-core/video/readers/jpeg.c index e5148120f..e31602f76 100644 --- a/grub-core/video/readers/jpeg.c +++ b/grub-core/video/readers/jpeg.c @@ -646,6 +646,10 @@ grub_jpeg_decode_data (struct grub_jpeg_data *data) nr1 = (data->image_height + vb - 1) >> (3 + data->log_vs); nc1 = (data->image_width + hb - 1) >> (3 + data->log_hs); + if (data->bitmap_ptr == NULL) +return grub_error(GRUB_ERR_BAD_FILE_TYPE, + "jpeg: attempted to decode data before start of stream"); + for (; data->r1 < nr1 && (!data->dri || rst); data->r1++, data->bitmap_ptr += (vb * data->image_width - hb * nc1) * 3) for (c1 = 0; c1 < nc1 && (!data->dri || rst); -- 2.11.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 039/117] zfsinfo: Correct a check for error allocating memory
From: Darren Kenny
While arguably the check for grub_errno is correct, we should really be
checking the return value from the function since it is always possible
that grub_errno was set elsewhere, making this code behave incorrectly.
Fixes: CID 73668
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
grub-core/fs/zfs/zfsinfo.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/grub-core/fs/zfs/zfsinfo.c b/grub-core/fs/zfs/zfsinfo.c
index c8a28acf5..bf2918018 100644
--- a/grub-core/fs/zfs/zfsinfo.c
+++ b/grub-core/fs/zfs/zfsinfo.c
@@ -358,8 +358,8 @@ grub_cmd_zfs_bootfs (grub_command_t cmd __attribute__
((unused)), int argc,
return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("one argument expected"));
devname = grub_file_get_device_name (args[0]);
- if (grub_errno)
-return grub_errno;
+ if (devname == NULL)
+return GRUB_ERR_OUT_OF_MEMORY;
dev = grub_device_open (devname);
grub_free (devname);
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 095/117] kern/parser: Fix a memory leak
From: Chris Coulson
The getline() function supplied to grub_parser_split_cmdline() returns
a newly allocated buffer and can be called multiple times, but the
returned buffer is never freed.
Signed-off-by: Chris Coulson
Reviewed-by: Daniel Kiper
---
grub-core/kern/parser.c | 20
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/grub-core/kern/parser.c b/grub-core/kern/parser.c
index d1cf061ad..39e4df65b 100644
--- a/grub-core/kern/parser.c
+++ b/grub-core/kern/parser.c
@@ -140,6 +140,7 @@ grub_parser_split_cmdline (const char *cmdline,
char buffer[1024];
char *bp = buffer;
char *rd = (char *) cmdline;
+ char *rp = rd;
char varname[200];
char *vp = varname;
char *args;
@@ -149,10 +150,18 @@ grub_parser_split_cmdline (const char *cmdline,
*argv = NULL;
do
{
- if (!rd || !*rd)
+ if (rp == NULL || *rp == '\0')
{
+ if (rd != cmdline)
+ {
+ grub_free (rd);
+ rd = rp = NULL;
+ }
if (getline)
- getline (&rd, 1, getline_data);
+ {
+ getline (&rd, 1, getline_data);
+ rp = rd;
+ }
else
break;
}
@@ -160,12 +169,12 @@ grub_parser_split_cmdline (const char *cmdline,
if (!rd)
break;
- for (; *rd; rd++)
+ for (; *rp != '\0'; rp++)
{
grub_parser_state_t newstate;
char use;
- newstate = grub_parser_cmdline_state (state, *rd, &use);
+ newstate = grub_parser_cmdline_state (state, *rp, &use);
/* If a variable was being processed and this character does
not describe the variable anymore, write the variable to
@@ -198,6 +207,9 @@ grub_parser_split_cmdline (const char *cmdline,
}
while (state != GRUB_PARSER_STATE_TEXT && !check_varstate (state));
+ if (rd != cmdline)
+grub_free (rd);
+
/* A special case for when the last character was part of a
variable. */
add_var (varname, &bp, &vp, state, GRUB_PARSER_STATE_TEXT);
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 068/117] video/readers/jpeg: Catch OOB reads/writes in grub_jpeg_decode_du()
From: Daniel Axtens
The key line is:
du[jpeg_zigzag_order[pos]] = val * (int) data->quan_table[qt][pos];
jpeg_zigzag_order is grub_uint8_t[64].
I don't understand JPEG decoders quite well enough to explain what's
going on here. However, I observe sometimes pos=64, which leads to an
OOB read of the jpeg_zigzag_order global then an OOB write to du.
That leads to various unpleasant memory corruption conditions.
Catch where pos >= ARRAY_SIZE(jpeg_zigzag_order) and bail.
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/video/readers/jpeg.c | 8
1 file changed, 8 insertions(+)
diff --git a/grub-core/video/readers/jpeg.c b/grub-core/video/readers/jpeg.c
index 23f919aa0..e5148120f 100644
--- a/grub-core/video/readers/jpeg.c
+++ b/grub-core/video/readers/jpeg.c
@@ -526,6 +526,14 @@ grub_jpeg_decode_du (struct grub_jpeg_data *data, int id,
jpeg_data_unit_t du)
val = grub_jpeg_get_number (data, num & 0xF);
num >>= 4;
pos += num;
+
+ if (pos >= ARRAY_SIZE (jpeg_zigzag_order))
+ {
+ grub_error (GRUB_ERR_BAD_FILE_TYPE,
+ "jpeg: invalid position in zigzag order!?");
+ return;
+ }
+
du[jpeg_zigzag_order[pos]] = val * (int) data->quan_table[qt][pos];
pos++;
}
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 038/117] zfs: Fix possible integer overflows
From: Darren Kenny
In all cases the problem is that the value being acted upon by
a left-shift is a 32-bit number which is then being used in the
context of a 64-bit number.
To avoid overflow we ensure that the number being shifted is 64-bit
before the shift is done.
Fixes: CID 73684, CID 73695, CID 73764
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
grub-core/fs/zfs/zfs.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/grub-core/fs/zfs/zfs.c b/grub-core/fs/zfs/zfs.c
index ca82eb878..9ef2fd61a 100644
--- a/grub-core/fs/zfs/zfs.c
+++ b/grub-core/fs/zfs/zfs.c
@@ -567,7 +567,7 @@ find_bestub (uberblock_phys_t * ub_array,
ubptr = (uberblock_phys_t *) ((grub_properly_aligned_t *) ub_array
+ ((i << ub_shift)
/ sizeof (grub_properly_aligned_t)));
- err = uberblock_verify (ubptr, offset, 1 << ub_shift);
+ err = uberblock_verify (ubptr, offset, (grub_size_t) 1 << ub_shift);
if (err)
{
grub_errno = GRUB_ERR_NONE;
@@ -1546,7 +1546,7 @@ read_device (grub_uint64_t offset, struct
grub_zfs_device_desc *desc,
high = grub_divmod64 ((offset >> desc->ashift) + c,
desc->n_children, &devn);
- csize = bsize << desc->ashift;
+ csize = (grub_size_t) bsize << desc->ashift;
if (csize > len)
csize = len;
@@ -1638,8 +1638,8 @@ read_device (grub_uint64_t offset, struct
grub_zfs_device_desc *desc,
while (len > 0)
{
- grub_size_t csize;
- csize = ((s / (desc->n_children - desc->nparity))
+ grub_size_t csize = s;
+ csize = ((csize / (desc->n_children - desc->nparity))
<< desc->ashift);
if (csize > len)
csize = len;
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 094/117] fs/btrfs: Squash some uninitialized reads
From: Daniel Axtens
We need to check errors before calling into a function that uses the result.
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/fs/btrfs.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
index c4ba5f110..63203034d 100644
--- a/grub-core/fs/btrfs.c
+++ b/grub-core/fs/btrfs.c
@@ -383,9 +383,9 @@ next (struct grub_btrfs_data *data,
err = grub_btrfs_read_logical (data, grub_le_to_cpu64 (node.addr),
&head, sizeof (head), 0);
+ if (err)
+ return -err;
check_btrfs_header (data, &head, grub_le_to_cpu64 (node.addr));
- if (err)
- return -err;
save_ref (desc, grub_le_to_cpu64 (node.addr), 0,
grub_le_to_cpu32 (head.nitems), !head.level);
@@ -445,9 +445,9 @@ lower_bound (struct grub_btrfs_data *data,
/* FIXME: preread few nodes into buffer. */
err = grub_btrfs_read_logical (data, addr, &head, sizeof (head),
recursion_depth + 1);
+ if (err)
+ return err;
check_btrfs_header (data, &head, addr);
- if (err)
- return err;
addr += sizeof (head);
if (head.level)
{
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 075/117] fs/sfs: Fix over-read of root object name
From: Daniel Axtens There's a read of the name of the root object that assumes that the name is nul-terminated within the root block. This isn't guaranteed - it seems SFS would require you to read multiple blocks to get a full name in general, but maybe that doesn't apply to the root object. Either way, figure out how much space is left in the root block and don't over-read it. This fixes some OOB reads. Signed-off-by: Daniel Axtens Reviewed-by: Daniel Kiper --- grub-core/fs/sfs.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/grub-core/fs/sfs.c b/grub-core/fs/sfs.c index de2b107a4..983e88008 100644 --- a/grub-core/fs/sfs.c +++ b/grub-core/fs/sfs.c @@ -373,6 +373,7 @@ grub_sfs_mount (grub_disk_t disk) struct grub_sfs_objc *rootobjc; char *rootobjc_data = 0; grub_uint32_t blk; + unsigned int max_len; data = grub_malloc (sizeof (*data)); if (!data) @@ -421,7 +422,13 @@ grub_sfs_mount (grub_disk_t disk) data->diropen.data = data; data->diropen.cache = 0; data->disk = disk; - data->label = grub_strdup ((char *) (rootobjc->objects[0].filename)); + + /* We only read 1 block of data, so truncate the name if needed. */ + max_len = ((GRUB_DISK_SECTOR_SIZE << data->log_blocksize) +- 24/* offsetof (struct grub_sfs_objc, objects) */ +- 25); /* offsetof (struct grub_sfs_obj, filename) */ + data->label = grub_zalloc (max_len + 1); + grub_strncpy (data->label, (char *) rootobjc->objects[0].filename, max_len); grub_free (rootobjc_data); return data; -- 2.11.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 100/117] kern/parser: Fix a stack buffer overflow
From: Chris Coulson
grub_parser_split_cmdline() expands variable names present in the supplied
command line in to their corresponding variable contents and uses a 1 kiB
stack buffer for temporary storage without sufficient bounds checking. If
the function is called with a command line that references a variable with
a sufficiently large payload, it is possible to overflow the stack
buffer via tab completion, corrupt the stack frame and potentially
control execution.
Fixes: CVE-2020-27749
Reported-by: Chris Coulson
Signed-off-by: Chris Coulson
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
grub-core/kern/parser.c | 110 +---
1 file changed, 67 insertions(+), 43 deletions(-)
diff --git a/grub-core/kern/parser.c b/grub-core/kern/parser.c
index e010eaa1f..6ab7aa427 100644
--- a/grub-core/kern/parser.c
+++ b/grub-core/kern/parser.c
@@ -18,6 +18,7 @@
*/
#include
+#include
#include
#include
#include
@@ -107,8 +108,8 @@ check_varstate (grub_parser_state_t s)
}
-static void
-add_var (char *varname, char **bp, char **vp,
+static grub_err_t
+add_var (grub_buffer_t varname, grub_buffer_t buf,
grub_parser_state_t state, grub_parser_state_t newstate)
{
const char *val;
@@ -116,31 +117,41 @@ add_var (char *varname, char **bp, char **vp,
/* Check if a variable was being read in and the end of the name
was reached. */
if (!(check_varstate (state) && !check_varstate (newstate)))
-return;
+return GRUB_ERR_NONE;
- *((*vp)++) = '\0';
- val = grub_env_get (varname);
- *vp = varname;
+ if (grub_buffer_append_char (varname, '\0') != GRUB_ERR_NONE)
+return grub_errno;
+
+ val = grub_env_get ((const char *) grub_buffer_peek_data (varname));
+ grub_buffer_reset (varname);
if (!val)
-return;
+return GRUB_ERR_NONE;
/* Insert the contents of the variable in the buffer. */
- for (; *val; val++)
-*((*bp)++) = *val;
+ return grub_buffer_append_data (buf, val, grub_strlen (val));
}
-static void
-terminate_arg (char *buffer, char **bp, int *argc)
+static grub_err_t
+terminate_arg (grub_buffer_t buffer, int *argc)
{
- if (*bp != buffer && *((*bp) - 1) != '\0')
-{
- *((*bp)++) = '\0';
- (*argc)++;
-}
+ grub_size_t unread = grub_buffer_get_unread_bytes (buffer);
+
+ if (unread == 0)
+return GRUB_ERR_NONE;
+
+ if (*(const char *) grub_buffer_peek_data_at (buffer, unread - 1) == '\0')
+return GRUB_ERR_NONE;
+
+ if (grub_buffer_append_char (buffer, '\0') != GRUB_ERR_NONE)
+return grub_errno;
+
+ (*argc)++;
+
+ return GRUB_ERR_NONE;
}
static grub_err_t
-process_char (char c, char *buffer, char **bp, char *varname, char **vp,
+process_char (char c, grub_buffer_t buffer, grub_buffer_t varname,
grub_parser_state_t state, int *argc,
grub_parser_state_t *newstate)
{
@@ -153,12 +164,13 @@ process_char (char c, char *buffer, char **bp, char
*varname, char **vp,
* not describe the variable anymore, write the variable to
* the buffer.
*/
- add_var (varname, bp, vp, state, *newstate);
+ if (add_var (varname, buffer, state, *newstate) != GRUB_ERR_NONE)
+return grub_errno;
if (check_varstate (*newstate))
{
if (use)
- *((*vp)++) = use;
+return grub_buffer_append_char (varname, use);
}
else if (*newstate == GRUB_PARSER_STATE_TEXT &&
state != GRUB_PARSER_STATE_ESC && grub_isspace (use))
@@ -167,10 +179,10 @@ process_char (char c, char *buffer, char **bp, char
*varname, char **vp,
* Don't add more than one argument if multiple
* spaces are used.
*/
- terminate_arg (buffer, bp, argc);
+ return terminate_arg (buffer, argc);
}
else if (use)
-*((*bp)++) = use;
+return grub_buffer_append_char (buffer, use);
return GRUB_ERR_NONE;
}
@@ -181,19 +193,22 @@ grub_parser_split_cmdline (const char *cmdline,
int *argc, char ***argv)
{
grub_parser_state_t state = GRUB_PARSER_STATE_TEXT;
- /* XXX: Fixed size buffer, perhaps this buffer should be dynamically
- allocated. */
- char buffer[1024];
- char *bp = buffer;
+ grub_buffer_t buffer, varname;
char *rd = (char *) cmdline;
char *rp = rd;
- char varname[200];
- char *vp = varname;
- char *args;
int i;
*argc = 0;
*argv = NULL;
+
+ buffer = grub_buffer_new (1024);
+ if (buffer == NULL)
+return grub_errno;
+
+ varname = grub_buffer_new (200);
+ if (varname == NULL)
+goto fail;
+
do
{
if (rp == NULL || *rp == '\0')
@@ -219,7 +234,7 @@ grub_parser_split_cmdline (const char *cmdline,
{
grub_parser_state_t newstate;
- if (process_char (*rp, buffer, &bp, varname, &vp, state, argc,
+ if (process_char (*rp, buffer, varname, state, argc,
&newstate) != GRUB_ERR_NONE)
goto fail;
@@ -230,10 +245,12 @@ grub_parser_split_cmdline (const
[SECURITY PATCH 040/117] affs: Fix memory leaks
From: Darren Kenny
The node structure reference is being allocated but not freed if it
reaches the end of the function. If any of the hooks had returned
a non-zero value, then node would have been copied in to the context
reference, but otherwise node is not stored and should be freed.
Similarly, the call to grub_affs_create_node() replaces the allocated
memory in node with a newly allocated structure, leaking the existing
memory pointed by node.
Finally, when dir->parent is set, then we again replace node with newly
allocated memory, which seems unnecessary when we copy in the values
from dir->parent immediately after.
Fixes: CID 73759
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
grub-core/fs/affs.c | 18 --
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/grub-core/fs/affs.c b/grub-core/fs/affs.c
index 220b3712f..230e26af0 100644
--- a/grub-core/fs/affs.c
+++ b/grub-core/fs/affs.c
@@ -400,12 +400,12 @@ grub_affs_iterate_dir (grub_fshelp_node_t dir,
{
unsigned int i;
struct grub_affs_file file;
- struct grub_fshelp_node *node = 0;
+ struct grub_fshelp_node *node, *orig_node;
struct grub_affs_data *data = dir->data;
grub_uint32_t *hashtable;
/* Create the directory entries for `.' and `..'. */
- node = grub_zalloc (sizeof (*node));
+ node = orig_node = grub_zalloc (sizeof (*node));
if (!node)
return 1;
@@ -414,9 +414,6 @@ grub_affs_iterate_dir (grub_fshelp_node_t dir,
return 1;
if (dir->parent)
{
- node = grub_zalloc (sizeof (*node));
- if (!node)
- return 1;
*node = *dir->parent;
if (hook ("..", GRUB_FSHELP_DIR, node, hook_data))
return 1;
@@ -456,17 +453,18 @@ grub_affs_iterate_dir (grub_fshelp_node_t dir,
if (grub_affs_create_node (dir, hook, hook_data, &node, &hashtable,
next, &file))
- return 1;
+ {
+ /* Node has been replaced in function. */
+ grub_free (orig_node);
+ return 1;
+ }
next = grub_be_to_cpu32 (file.next);
}
}
- grub_free (hashtable);
- return 0;
-
fail:
- grub_free (node);
+ grub_free (orig_node);
grub_free (hashtable);
return 0;
}
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 001/117] verifiers: Move verifiers API to kernel image
From: Marco A Benatto
Move verifiers API from a module to the kernel image, so it can be
used there as well. There are no functional changes in this patch.
Signed-off-by: Marco A Benatto
Signed-off-by: Javier Martinez Canillas
Reviewed-by: Daniel Kiper
---
grub-core/Makefile.am| 1 +
grub-core/Makefile.core.def | 6 +-
grub-core/kern/main.c| 4
grub-core/{commands => kern}/verifiers.c | 8 ++--
include/grub/verify.h| 9 ++---
5 files changed, 14 insertions(+), 14 deletions(-)
rename grub-core/{commands => kern}/verifiers.c (97%)
diff --git a/grub-core/Makefile.am b/grub-core/Makefile.am
index c6ba5b2d7..cc6fc7dfa 100644
--- a/grub-core/Makefile.am
+++ b/grub-core/Makefile.am
@@ -91,6 +91,7 @@ KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/parser.h
KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/partition.h
KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/term.h
KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/time.h
+KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/verify.h
KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/mm_private.h
KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/net.h
KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/memory.h
diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index 68b9e9f68..0cecc3875 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -140,6 +140,7 @@ kernel = {
common = kern/rescue_parser.c;
common = kern/rescue_reader.c;
common = kern/term.c;
+ common = kern/verifiers.c;
noemu = kern/compiler-rt.c;
noemu = kern/mm.c;
@@ -944,11 +945,6 @@ module = {
};
module = {
- name = verifiers;
- common = commands/verifiers.c;
-};
-
-module = {
name = shim_lock;
common = commands/efi/shim_lock.c;
enable = efi;
diff --git a/grub-core/kern/main.c b/grub-core/kern/main.c
index 9cad0c448..73967e2f5 100644
--- a/grub-core/kern/main.c
+++ b/grub-core/kern/main.c
@@ -29,6 +29,7 @@
#include
#include
#include
+#include
#ifdef GRUB_MACHINE_PCBIOS
#include
@@ -274,6 +275,9 @@ grub_main (void)
grub_printf ("Welcome to GRUB!\n\n");
grub_setcolorstate (GRUB_TERM_COLOR_STANDARD);
+ /* Init verifiers API. */
+ grub_verifiers_init ();
+
grub_load_config ();
grub_boot_time ("Before loading embedded modules.");
diff --git a/grub-core/commands/verifiers.c b/grub-core/kern/verifiers.c
similarity index 97%
rename from grub-core/commands/verifiers.c
rename to grub-core/kern/verifiers.c
index aae4d84bb..75d7994cf 100644
--- a/grub-core/commands/verifiers.c
+++ b/grub-core/kern/verifiers.c
@@ -221,12 +221,8 @@ grub_verify_string (char *str, enum
grub_verify_string_type type)
return GRUB_ERR_NONE;
}
-GRUB_MOD_INIT(verifiers)
+void
+grub_verifiers_init (void)
{
grub_file_filter_register (GRUB_FILE_FILTER_VERIFY, grub_verifiers_open);
}
-
-GRUB_MOD_FINI(verifiers)
-{
- grub_file_filter_unregister (GRUB_FILE_FILTER_VERIFY);
-}
diff --git a/include/grub/verify.h b/include/grub/verify.h
index ea0491433..cd129c398 100644
--- a/include/grub/verify.h
+++ b/include/grub/verify.h
@@ -64,7 +64,10 @@ struct grub_file_verifier
grub_err_t (*verify_string) (char *str, enum grub_verify_string_type type);
};
-extern struct grub_file_verifier *grub_file_verifiers;
+extern struct grub_file_verifier *EXPORT_VAR (grub_file_verifiers);
+
+extern void
+grub_verifiers_init (void);
static inline void
grub_verifier_register (struct grub_file_verifier *ver)
@@ -78,7 +81,7 @@ grub_verifier_unregister (struct grub_file_verifier *ver)
grub_list_remove (GRUB_AS_LIST (ver));
}
-grub_err_t
-grub_verify_string (char *str, enum grub_verify_string_type type);
+extern grub_err_t
+EXPORT_FUNC (grub_verify_string) (char *str, enum grub_verify_string_type
type);
#endif /* ! GRUB_VERIFY_HEADER */
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 089/117] disk/lvm: Do not crash if an expected string is not found
From: Daniel Axtens
Clean up a bunch of cases where we could have strstr() fail and lead to
us dereferencing NULL.
We'll still leak memory in some cases (loops don't clean up allocations
from earlier iterations if a later iteration fails) but at least we're
not crashing.
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/disk/lvm.c | 22 +-
1 file changed, 17 insertions(+), 5 deletions(-)
diff --git a/grub-core/disk/lvm.c b/grub-core/disk/lvm.c
index 31bbc9acc..201097fda 100644
--- a/grub-core/disk/lvm.c
+++ b/grub-core/disk/lvm.c
@@ -578,7 +578,16 @@ grub_lvm_detect (grub_disk_t disk,
}
if (seg->node_count != 1)
- seg->stripe_size = grub_lvm_getvalue (&p, "stripe_size
= ");
+ {
+ seg->stripe_size = grub_lvm_getvalue (&p,
"stripe_size = ");
+ if (p == NULL)
+ {
+#ifdef GRUB_UTIL
+ grub_util_info ("unknown stripe_size");
+#endif
+ goto lvs_segment_fail;
+ }
+ }
seg->nodes = grub_calloc (seg->node_count,
sizeof (*stripe));
@@ -598,7 +607,7 @@ grub_lvm_detect (grub_disk_t disk,
{
p = grub_strchr (p, '"');
if (p == NULL)
- continue;
+ goto lvs_segment_fail2;
q = ++p;
while (*q != '"')
q++;
@@ -617,7 +626,10 @@ grub_lvm_detect (grub_disk_t disk,
stripe->start = grub_lvm_getvalue (&p, ",")
* vg->extent_size;
if (p == NULL)
- continue;
+ {
+ grub_free (stripe->name);
+ goto lvs_segment_fail2;
+ }
stripe++;
}
@@ -654,7 +666,7 @@ grub_lvm_detect (grub_disk_t disk,
p = grub_strchr (p, '"');
if (p == NULL)
- continue;
+ goto lvs_segment_fail2;
q = ++p;
while (*q != '"')
q++;
@@ -742,7 +754,7 @@ grub_lvm_detect (grub_disk_t disk,
p = p ? grub_strchr (p + 1, '"') : 0;
p = p ? grub_strchr (p + 1, '"') : 0;
if (p == NULL)
- continue;
+ goto lvs_segment_fail2;
q = ++p;
while (*q != '"')
q++;
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 101/117] kern/efi: Add initial stack protector implementation
From: Chris Coulson
It works only on UEFI platforms but can be quite easily extended to
others architectures and platforms if needed.
Signed-off-by: Chris Coulson
Signed-off-by: Daniel Kiper
Reviewed-by: Marco A Benatto
Reviewed-by: Javier Martinez Canillas
---
acinclude.m4 | 38 +++--
configure.ac | 44 ++
grub-core/Makefile.am | 1 +
grub-core/kern/efi/init.c | 54 ++
include/grub/efi/api.h | 19 +++
include/grub/stack_protector.h | 30 +++
6 files changed, 179 insertions(+), 7 deletions(-)
create mode 100644 include/grub/stack_protector.h
diff --git a/acinclude.m4 b/acinclude.m4
index 78cdf6e1d..6e14bb553 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -305,9 +305,9 @@ fi
])
-dnl Check if the C compiler supports `-fstack-protector'.
+dnl Check if the C compiler supports the stack protector
AC_DEFUN([grub_CHECK_STACK_PROTECTOR],[
-[# Smashing stack protector.
+[# Stack smashing protector.
ssp_possible=yes]
AC_MSG_CHECKING([whether `$CC' accepts `-fstack-protector'])
# Is this a reliable test case?
@@ -324,6 +324,40 @@ else
ssp_possible=no]
AC_MSG_RESULT([no])
[fi]
+[# Strong stack smashing protector.
+ssp_strong_possible=yes]
+AC_MSG_CHECKING([whether `$CC' accepts `-fstack-protector-strong'])
+# Is this a reliable test case?
+AC_LANG_CONFTEST([AC_LANG_SOURCE([[
+void foo (void) { volatile char a[8]; a[3]; }
+]])])
+[# `$CC -c -o ...' might not be portable. But, oh, well... Is calling
+# `ac_compile' like this correct, after all?
+if eval "$ac_compile -S -fstack-protector-strong -o conftest.s" 2> /dev/null;
then]
+ AC_MSG_RESULT([yes])
+ [# Should we clear up other files as well, having called `AC_LANG_CONFTEST'?
+ rm -f conftest.s
+else
+ ssp_strong_possible=no]
+ AC_MSG_RESULT([no])
+[fi]
+[# Global stack smashing protector.
+ssp_global_possible=yes]
+AC_MSG_CHECKING([whether `$CC' accepts `-mstack-protector-guard=global'])
+# Is this a reliable test case?
+AC_LANG_CONFTEST([AC_LANG_SOURCE([[
+void foo (void) { volatile char a[8]; a[3]; }
+]])])
+[# `$CC -c -o ...' might not be portable. But, oh, well... Is calling
+# `ac_compile' like this correct, after all?
+if eval "$ac_compile -S -fstack-protector -mstack-protector-guard=global -o
conftest.s" 2> /dev/null; then]
+ AC_MSG_RESULT([yes])
+ [# Should we clear up other files as well, having called `AC_LANG_CONFTEST'?
+ rm -f conftest.s
+else
+ ssp_global_possible=no]
+ AC_MSG_RESULT([no])
+[fi]
])
dnl Check if the C compiler supports `-mstack-arg-probe' (Cygwin).
diff --git a/configure.ac b/configure.ac
index 7c10a4db7..676137deb 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1312,12 +1312,41 @@ fi]
CFLAGS="$TARGET_CFLAGS"
-# Smashing stack protector.
+# Stack smashing protector.
grub_CHECK_STACK_PROTECTOR
-# Need that, because some distributions ship compilers that include
-# `-fstack-protector' in the default specs.
-if test "x$ssp_possible" = xyes; then
- TARGET_CFLAGS="$TARGET_CFLAGS -fno-stack-protector"
+AC_ARG_ENABLE([stack-protector],
+ AS_HELP_STRING([--enable-stack-protector],
+[enable the stack protector]),
+ [],
+ [enable_stack_protector=no])
+if test "x$enable_stack_protector" = xno; then
+ if test "x$ssp_possible" = xyes; then
+# Need that, because some distributions ship compilers that include
+# `-fstack-protector' in the default specs.
+TARGET_CFLAGS="$TARGET_CFLAGS -fno-stack-protector"
+ fi
+elif test "x$platform" != xefi; then
+ AC_MSG_ERROR([--enable-stack-protector is only supported on EFI platforms])
+elif test "x$ssp_global_possible" != xyes; then
+ AC_MSG_ERROR([--enable-stack-protector is not supported (compiler doesn't
support -mstack-protector-guard=global)])
+else
+ TARGET_CFLAGS="$TARGET_CFLAGS -mstack-protector-guard=global"
+ if test "x$enable_stack_protector" = xyes; then
+if test "x$ssp_possible" != xyes; then
+ AC_MSG_ERROR([--enable-stack-protector is not supported (compiler
doesn't support -fstack-protector)])
+fi
+TARGET_CFLAGS="$TARGET_CFLAGS -fstack-protector"
+ elif test "x$enable_stack_protector" = xstrong; then
+if test "x$ssp_strong_possible" != xyes; then
+ AC_MSG_ERROR([--enable-stack-protector=strong is not supported (compiler
doesn't support -fstack-protector-strong)])
+fi
+TARGET_CFLAGS="$TARGET_CFLAGS -fstack-protector-strong"
+ else
+# Note, -fstack-protector-all requires that the protector is disabled for
+# functions that appear in the call stack when the canary is initialized.
+AC_MSG_ERROR([invalid value $enable_stack_protector for
--enable-stack-protector])
+ fi
+ TARGET_CPPFLAGS="$TARGET_CPPFLAGS -DGRUB_STACK_PROTECTOR=1"
fi
CFLAGS="$TARGET_CFLAGS"
@@ -2132,5 +2161,10 @@ echo "Without liblzma (no support for XZ-compresse
[SECURITY PATCH 047/117] video/efi_gop: Remove unnecessary return value of grub_video_gop_fill_mode_info()
From: Darren Kenny
The return value of grub_video_gop_fill_mode_info() is never able to be
anything other than GRUB_ERR_NONE. So, rather than continue to return
a value and checking it each time, it is more correct to redefine the
function to not return anything and remove checks of its return value
altogether.
Fixes: CID 96701
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
grub-core/video/efi_gop.c | 25 ++---
1 file changed, 6 insertions(+), 19 deletions(-)
diff --git a/grub-core/video/efi_gop.c b/grub-core/video/efi_gop.c
index 7fe0cdabf..b7590dc6c 100644
--- a/grub-core/video/efi_gop.c
+++ b/grub-core/video/efi_gop.c
@@ -235,7 +235,7 @@ grub_video_gop_fill_real_mode_info (unsigned mode,
return GRUB_ERR_NONE;
}
-static grub_err_t
+static void
grub_video_gop_fill_mode_info (unsigned mode,
struct grub_efi_gop_mode_info *in,
struct grub_video_mode_info *out)
@@ -260,8 +260,6 @@ grub_video_gop_fill_mode_info (unsigned mode,
out->blit_format = GRUB_VIDEO_BLIT_FORMAT_BGRA_;
out->mode_type |= (GRUB_VIDEO_MODE_TYPE_DOUBLE_BUFFERED
| GRUB_VIDEO_MODE_TYPE_UPDATING_SWAP);
-
- return GRUB_ERR_NONE;
}
static int
@@ -274,7 +272,6 @@ grub_video_gop_iterate (int (*hook) (const struct
grub_video_mode_info *info, vo
grub_efi_uintn_t size;
grub_efi_status_t status;
struct grub_efi_gop_mode_info *info = NULL;
- grub_err_t err;
struct grub_video_mode_info mode_info;
status = efi_call_4 (gop->query_mode, gop, mode, &size, &info);
@@ -285,12 +282,7 @@ grub_video_gop_iterate (int (*hook) (const struct
grub_video_mode_info *info, vo
continue;
}
- err = grub_video_gop_fill_mode_info (mode, info, &mode_info);
- if (err)
- {
- grub_errno = GRUB_ERR_NONE;
- continue;
- }
+ grub_video_gop_fill_mode_info (mode, info, &mode_info);
if (hook (&mode_info, hook_arg))
return 1;
}
@@ -474,13 +466,8 @@ grub_video_gop_setup (unsigned int width, unsigned int
height,
info = gop->mode->info;
- err = grub_video_gop_fill_mode_info (gop->mode->mode, info,
- &framebuffer.mode_info);
- if (err)
-{
- grub_dprintf ("video", "GOP: couldn't fill mode info\n");
- return err;
-}
+ grub_video_gop_fill_mode_info (gop->mode->mode, info,
+&framebuffer.mode_info);
framebuffer.ptr = (void *) (grub_addr_t) gop->mode->fb_base;
framebuffer.offscreen
@@ -494,8 +481,8 @@ grub_video_gop_setup (unsigned int width, unsigned int
height,
{
grub_dprintf ("video", "GOP: couldn't allocate shadow\n");
grub_errno = 0;
- err = grub_video_gop_fill_mode_info (gop->mode->mode, info,
- &framebuffer.mode_info);
+ grub_video_gop_fill_mode_info (gop->mode->mode, info,
+&framebuffer.mode_info);
buffer = framebuffer.ptr;
}
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 002/117] efi: Move the shim_lock verifier to the GRUB core
From: Marco A Benatto
Move the shim_lock verifier from its own module into the core image. The
Secure Boot lockdown mechanism has the intent to prevent the load of any
unsigned code or binary when Secure Boot is enabled.
The reason is that GRUB must be able to prevent executing untrusted code
if UEFI Secure Boot is enabled, without depending on external modules.
Signed-off-by: Marco A Benatto
Signed-off-by: Javier Martinez Canillas
Reviewed-by: Daniel Kiper
---
docs/grub.texi | 9 +--
grub-core/Makefile.core.def| 6 --
grub-core/commands/efi/shim_lock.c | 133 -
grub-core/kern/efi/init.c | 4 ++
grub-core/kern/efi/sb.c| 105 +
include/grub/efi/sb.h | 3 +
6 files changed, 117 insertions(+), 143 deletions(-)
delete mode 100644 grub-core/commands/efi/shim_lock.c
diff --git a/docs/grub.texi b/docs/grub.texi
index eeac9b2ce..e3de2a4dc 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -5910,15 +5910,16 @@ secure boot chain.
@section UEFI secure boot and shim support
The GRUB, except the @command{chainloader} command, works with the UEFI secure
-boot and the shim. This functionality is provided by the shim_lock module. It
-is recommend to build in this and other required modules into the
@file{core.img}.
+boot and the shim. This functionality is provided by the shim_lock verifier. It
+is built into the @file{core.img} and is registered if the UEFI secure boot is
+enabled.
+
All modules not stored in the @file{core.img} and the ACPI tables for the
@command{acpi} command have to be signed, e.g. using PGP. Additionally, the
@command{iorw}, the @command{memrw} and the @command{wrmsr} commands are
prohibited if the UEFI secure boot is enabled. This is done due to
security reasons. All above mentioned requirements are enforced by the
-shim_lock module. And itself it is a persistent module which means that
-it cannot be unloaded if it was loaded into the memory.
+shim_lock verifier logic.
@node Measured Boot
@section Measuring boot components
diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index 0cecc3875..2b98fe1fc 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -945,12 +945,6 @@ module = {
};
module = {
- name = shim_lock;
- common = commands/efi/shim_lock.c;
- enable = efi;
-};
-
-module = {
name = hdparm;
common = commands/hdparm.c;
enable = pci;
diff --git a/grub-core/commands/efi/shim_lock.c
b/grub-core/commands/efi/shim_lock.c
deleted file mode 100644
index f7f3109d6..0
--- a/grub-core/commands/efi/shim_lock.c
+++ /dev/null
@@ -1,133 +0,0 @@
-/*
- * GRUB -- GRand Unified Bootloader
- * Copyright (C) 2017 Free Software Foundation, Inc.
- *
- * GRUB is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * GRUB is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with GRUB. If not, see
[SECURITY PATCH 109/117] util/mkimage: Add an option to import SBAT metadata into a .sbat section
From: Peter Jones
Add a --sbat option to the grub-mkimage tool which allows us to import
an SBAT metadata formatted as a CSV file into a .sbat section of the
EFI binary.
Signed-off-by: Peter Jones
Signed-off-by: Javier Martinez Canillas
Reviewed-by: Daniel Kiper
---
docs/grub.texi | 19 +++
include/grub/util/install.h | 3 ++-
include/grub/util/mkimage.h | 1 +
util/grub-install-common.c | 2 +-
util/grub-mkimage.c | 15 ++-
util/mkimage.c | 43 ---
6 files changed, 73 insertions(+), 10 deletions(-)
diff --git a/docs/grub.texi b/docs/grub.texi
index c5fc3904f..81b90947a 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -5784,6 +5784,7 @@ environment variables and commands are listed in the same
order.
* Authentication and authorisation:: Users and access control
* Using digital signatures:: Booting digitally signed code
* UEFI secure boot and shim::Booting digitally signed PE files
+* Secure Boot Advanced Targeting:: Embedded information for generation
number based revocation
* Measured Boot::Measuring boot components
* Lockdown:: Lockdown when booting on a secure setup
@end menu
@@ -5958,6 +5959,24 @@ and @command{memrw} will not be available when the UEFI
secure boot is enabled.
This is done for security reasons and are enforced by the GRUB Lockdown
mechanism
(@pxref{Lockdown}).
+@node Secure Boot Advanced Targeting
+@section Embedded information for generation number based revocation
+
+The Secure Boot Advanced Targeting (SBAT) is a mechanism to allow the
revocation
+of components in the boot path by using generation numbers embedded into the
EFI
+binaries. The SBAT metadata is located in an .sbat data section that has set of
+UTF-8 strings as comma-separated values (CSV). See
+@uref{https://github.com/rhboot/shim/blob/main/SBAT.md} for more details.
+
+To add a data section containing the SBAT information into the binary, the
+@option{--sbat} option of @command{grub-mkimage} command should be used. The
content
+of a CSV file, encoded with UTF-8, is copied as is to the .sbat data section
into
+the generated EFI binary. The CSV file can be stored anywhere on the file
system.
+
+@example
+grub-mkimage -O x86_64-efi -o grubx64.efi -p '(tftp)/grub' --sbat sbat.csv
efinet tftp
+@end example
+
@node Measured Boot
@section Measuring boot components
diff --git a/include/grub/util/install.h b/include/grub/util/install.h
index 8cbbc4139..3736a16aa 100644
--- a/include/grub/util/install.h
+++ b/include/grub/util/install.h
@@ -183,7 +183,8 @@ grub_install_generate_image (const char *dir, const char
*prefix,
char *config_path,
const struct grub_install_image_target_desc
*image_target,
int note,
-grub_compression_t comp, const char *dtb_file);
+grub_compression_t comp, const char *dtb_file,
+const char *sbat_path);
const struct grub_install_image_target_desc *
grub_install_get_image_target (const char *arg);
diff --git a/include/grub/util/mkimage.h b/include/grub/util/mkimage.h
index ba9f568f6..3819a6744 100644
--- a/include/grub/util/mkimage.h
+++ b/include/grub/util/mkimage.h
@@ -24,6 +24,7 @@ struct grub_mkimage_layout
size_t exec_size;
size_t kernel_size;
size_t bss_size;
+ size_t sbat_size;
grub_uint64_t start_address;
void *reloc_section;
size_t reloc_size;
diff --git a/util/grub-install-common.c b/util/grub-install-common.c
index 08d3eb668..56dcb52bf 100644
--- a/util/grub-install-common.c
+++ b/util/grub-install-common.c
@@ -536,7 +536,7 @@ grub_install_make_image_wrap_file (const char *dir, const
char *prefix,
grub_install_generate_image (dir, prefix, fp, outname,
modules.entries, memdisk_path,
pubkeys, npubkeys, config_path, tgt,
- note, compression, dtb);
+ note, compression, dtb, NULL);
while (dc--)
grub_install_pop_module ();
}
diff --git a/util/grub-mkimage.c b/util/grub-mkimage.c
index 912564e36..75b884710 100644
--- a/util/grub-mkimage.c
+++ b/util/grub-mkimage.c
@@ -81,6 +81,7 @@ static struct argp_option options[] = {
{"output", 'o', N_("FILE"), 0, N_("output a generated image to FILE
[default=stdout]"), 0},
{"format", 'O', N_("FORMAT"), 0, 0, 0},
{"compression", 'C', "(xz|none|auto)", 0, N_("choose the compression to use
for core image"), 0},
+ {"sbat", 's', N_("FILE"), 0, N_("SBAT metadata"), 0},
{"verbose", 'v', 0, 0, N_("print verbose messages."), 0},
{ 0, 0, 0, 0, 0, 0 }
};
@@ -123,6 +124,7 @@ struct arguments
size_t npubkeys;
char *font;
char *config;
+ char *sbat;
int note;
const struct grub_install_image_target_desc *image_t
[SECURITY PATCH 096/117] kern/parser: Introduce process_char() helper
From: Chris Coulson
grub_parser_split_cmdline() iterates over each command line character.
In order to add error checking and to simplify the subsequent error
handling, split the character processing in to a separate function.
Signed-off-by: Chris Coulson
Reviewed-by: Daniel Kiper
---
grub-core/kern/parser.c | 74 ++---
1 file changed, 46 insertions(+), 28 deletions(-)
diff --git a/grub-core/kern/parser.c b/grub-core/kern/parser.c
index 39e4df65b..0d3582bd8 100644
--- a/grub-core/kern/parser.c
+++ b/grub-core/kern/parser.c
@@ -1,7 +1,7 @@
/* parser.c - the part of the parser that can return partial tokens */
/*
* GRUB -- GRand Unified Bootloader
- * Copyright (C) 2005,2007,2009 Free Software Foundation, Inc.
+ * Copyright (C) 2005,2007,2009,2021 Free Software Foundation, Inc.
*
* GRUB is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
@@ -129,6 +129,46 @@ add_var (char *varname, char **bp, char **vp,
*((*bp)++) = *val;
}
+static grub_err_t
+process_char (char c, char *buffer, char **bp, char *varname, char **vp,
+ grub_parser_state_t state, int *argc,
+ grub_parser_state_t *newstate)
+{
+ char use;
+
+ *newstate = grub_parser_cmdline_state (state, c, &use);
+
+ /*
+ * If a variable was being processed and this character does
+ * not describe the variable anymore, write the variable to
+ * the buffer.
+ */
+ add_var (varname, bp, vp, state, *newstate);
+
+ if (check_varstate (*newstate))
+{
+ if (use)
+ *((*vp)++) = use;
+}
+ else if (*newstate == GRUB_PARSER_STATE_TEXT &&
+ state != GRUB_PARSER_STATE_ESC && grub_isspace (use))
+{
+ /*
+ * Don't add more than one argument if multiple
+ * spaces are used.
+ */
+ if (*bp != buffer && *((*bp) - 1) != '\0')
+ {
+ *((*bp)++) = '\0';
+ (*argc)++;
+ }
+}
+ else if (use)
+*((*bp)++) = use;
+
+ return GRUB_ERR_NONE;
+}
+
grub_err_t
grub_parser_split_cmdline (const char *cmdline,
grub_reader_getline_t getline, void *getline_data,
@@ -172,35 +212,13 @@ grub_parser_split_cmdline (const char *cmdline,
for (; *rp != '\0'; rp++)
{
grub_parser_state_t newstate;
- char use;
- newstate = grub_parser_cmdline_state (state, *rp, &use);
-
- /* If a variable was being processed and this character does
-not describe the variable anymore, write the variable to
-the buffer. */
- add_var (varname, &bp, &vp, state, newstate);
-
- if (check_varstate (newstate))
- {
- if (use)
- *(vp++) = use;
- }
- else
+ if (process_char (*rp, buffer, &bp, varname, &vp, state, argc,
+ &newstate) != GRUB_ERR_NONE)
{
- if (newstate == GRUB_PARSER_STATE_TEXT
- && state != GRUB_PARSER_STATE_ESC && grub_isspace (use))
- {
- /* Don't add more than one argument if multiple
-spaces are used. */
- if (bp != buffer && *(bp - 1))
- {
- *(bp++) = '\0';
- (*argc)++;
- }
- }
- else if (use)
- *(bp++) = use;
+ if (rd != cmdline)
+ grub_free (rd);
+ return grub_errno;
}
state = newstate;
}
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 053/117] loader/bsd: Check for NULL arg up-front
From: Darren Kenny
The code in the next block suggests that it is possible for .set to be
true but .arg may still be NULL.
This code assumes that it is never NULL, yet later is testing if it is
NULL - that is inconsistent.
So we should check first if .arg is not NULL, and remove this check that
is being flagged by Coverity since it is no longer required.
Fixes: CID 292471
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
grub-core/loader/i386/bsd.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/grub-core/loader/i386/bsd.c b/grub-core/loader/i386/bsd.c
index f5bf7f89e..d89ff0a7a 100644
--- a/grub-core/loader/i386/bsd.c
+++ b/grub-core/loader/i386/bsd.c
@@ -1605,7 +1605,7 @@ grub_cmd_openbsd (grub_extcmd_context_t ctxt, int argc,
char *argv[])
kernel_type = KERNEL_TYPE_OPENBSD;
bootflags = grub_bsd_parse_flags (ctxt->state, openbsd_flags);
- if (ctxt->state[OPENBSD_ROOT_ARG].set)
+ if (ctxt->state[OPENBSD_ROOT_ARG].set && ctxt->state[OPENBSD_ROOT_ARG].arg
!= NULL)
{
const char *arg = ctxt->state[OPENBSD_ROOT_ARG].arg;
unsigned type, unit, part;
@@ -1622,7 +1622,7 @@ grub_cmd_openbsd (grub_extcmd_context_t ctxt, int argc,
char *argv[])
"unknown disk type name");
unit = grub_strtoul (arg, &arg, 10);
- if (! (arg && *arg >= 'a' && *arg <= 'z'))
+ if (! (*arg >= 'a' && *arg <= 'z'))
return grub_error (GRUB_ERR_BAD_ARGUMENT,
"only device specifications of form "
" are supported");
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 004/117] kern/lockdown: Set a variable if the GRUB is locked down
From: Javier Martinez Canillas
It may be useful for scripts to determine whether the GRUB is locked
down or not. Add the lockdown variable which is set to "y" when the GRUB
is locked down.
Suggested-by: Dimitri John Ledkov
Signed-off-by: Javier Martinez Canillas
Reviewed-by: Daniel Kiper
---
docs/grub.texi| 3 +++
grub-core/kern/lockdown.c | 4
2 files changed, 7 insertions(+)
diff --git a/docs/grub.texi b/docs/grub.texi
index fd3c78054..4c980d356 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -5966,6 +5966,9 @@ The GRUB can be locked down when booted on a secure boot
environment, for exampl
if the UEFI secure boot is enabled. On a locked down configuration, the GRUB
will
be restricted and some operations/commands cannot be executed.
+The @samp{lockdown} variable is set to @samp{y} when the GRUB is locked down.
+Otherwise it does not exit.
+
@node Platform limitations
@chapter Platform limitations
diff --git a/grub-core/kern/lockdown.c b/grub-core/kern/lockdown.c
index 1e56c0b80..0bc70fd42 100644
--- a/grub-core/kern/lockdown.c
+++ b/grub-core/kern/lockdown.c
@@ -18,6 +18,7 @@
*/
#include
+#include
#include
#include
#include
@@ -71,6 +72,9 @@ grub_lockdown (void)
lockdown = GRUB_LOCKDOWN_ENABLED;
grub_verifier_register (&lockdown_verifier);
+
+ grub_env_set ("lockdown", "y");
+ grub_env_export ("lockdown");
}
int
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 097/117] kern/parser: Introduce terminate_arg() helper
From: Chris Coulson
process_char() and grub_parser_split_cmdline() use similar code for
terminating the most recent argument. Add a helper function for this.
Signed-off-by: Chris Coulson
Reviewed-by: Daniel Kiper
---
grub-core/kern/parser.c | 23 +--
1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/grub-core/kern/parser.c b/grub-core/kern/parser.c
index 0d3582bd8..572c67089 100644
--- a/grub-core/kern/parser.c
+++ b/grub-core/kern/parser.c
@@ -129,6 +129,16 @@ add_var (char *varname, char **bp, char **vp,
*((*bp)++) = *val;
}
+static void
+terminate_arg (char *buffer, char **bp, int *argc)
+{
+ if (*bp != buffer && *((*bp) - 1) != '\0')
+{
+ *((*bp)++) = '\0';
+ (*argc)++;
+}
+}
+
static grub_err_t
process_char (char c, char *buffer, char **bp, char *varname, char **vp,
grub_parser_state_t state, int *argc,
@@ -157,11 +167,7 @@ process_char (char c, char *buffer, char **bp, char
*varname, char **vp,
* Don't add more than one argument if multiple
* spaces are used.
*/
- if (*bp != buffer && *((*bp) - 1) != '\0')
- {
- *((*bp)++) = '\0';
- (*argc)++;
- }
+ terminate_arg (buffer, bp, argc);
}
else if (use)
*((*bp)++) = use;
@@ -232,11 +238,8 @@ grub_parser_split_cmdline (const char *cmdline,
variable. */
add_var (varname, &bp, &vp, state, GRUB_PARSER_STATE_TEXT);
- if (bp != buffer && *(bp - 1))
-{
- *(bp++) = '\0';
- (*argc)++;
-}
+ /* Ensure that the last argument is terminated. */
+ terminate_arg (buffer, &bp, argc);
/* If there are no args, then we're done. */
if (!*argc)
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 056/117] loader/xnu: Check if pointer is NULL before using it
From: Paulo Flabiano Smorigo
Fixes: CID 73654
Signed-off-by: Paulo Flabiano Smorigo
Reviewed-by: Daniel Kiper
---
grub-core/loader/xnu.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/grub-core/loader/xnu.c b/grub-core/loader/xnu.c
index 1a590dbc0..1c0cf6a43 100644
--- a/grub-core/loader/xnu.c
+++ b/grub-core/loader/xnu.c
@@ -670,6 +670,9 @@ grub_xnu_load_driver (char *infoplistname, grub_file_t
binaryfile,
char *name, *nameend;
int namelen;
+ if (infoplistname == NULL)
+return grub_error (GRUB_ERR_BAD_FILENAME, N_("missing p-list filename"));
+
name = get_name_ptr (infoplistname);
nameend = grub_strchr (name, '/');
@@ -701,10 +704,7 @@ grub_xnu_load_driver (char *infoplistname, grub_file_t
binaryfile,
else
macho = 0;
- if (infoplistname)
-infoplist = grub_file_open (infoplistname, GRUB_FILE_TYPE_XNU_INFO_PLIST);
- else
-infoplist = 0;
+ infoplist = grub_file_open (infoplistname, GRUB_FILE_TYPE_XNU_INFO_PLIST);
grub_errno = GRUB_ERR_NONE;
if (infoplist)
{
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 003/117] kern: Add lockdown support
From: Javier Martinez Canillas
When the GRUB starts on a secure boot platform, some commands can be
used to subvert the protections provided by the verification mechanism and
could lead to booting untrusted system.
To prevent that situation, allow GRUB to be locked down. That way the code
may check if GRUB has been locked down and further restrict the commands
that are registered or what subset of their functionality could be used.
The lockdown support adds the following components:
* The grub_lockdown() function which can be used to lockdown GRUB if,
e.g., UEFI Secure Boot is enabled.
* The grub_is_lockdown() function which can be used to check if the GRUB
was locked down.
* A verifier that flags OS kernels, the GRUB modules, Device Trees and ACPI
tables as GRUB_VERIFY_FLAGS_DEFER_AUTH to defer verification to other
verifiers. These files are only successfully verified if another registered
verifier returns success. Otherwise, the whole verification process fails.
For example, PE/COFF binaries verification can be done by the shim_lock
verifier which validates the signatures using the shim_lock protocol.
However, the verification is not deferred directly to the shim_lock verifier.
The shim_lock verifier is hooked into the verification process instead.
* A set of grub_{command,extcmd}_lockdown functions that can be used by
code registering command handlers, to only register unsafe commands if
the GRUB has not been locked down.
Signed-off-by: Javier Martinez Canillas
Reviewed-by: Daniel Kiper
---
conf/Makefile.common| 2 ++
docs/grub-dev.texi | 27 +++
docs/grub.texi | 8 +
grub-core/Makefile.am | 5 ++-
grub-core/Makefile.core.def | 1 +
grub-core/commands/extcmd.c | 23 +
grub-core/kern/command.c| 24 ++
grub-core/kern/lockdown.c | 80 +
include/grub/command.h | 5 +++
include/grub/extcmd.h | 7
include/grub/lockdown.h | 44 +
11 files changed, 225 insertions(+), 1 deletion(-)
create mode 100644 grub-core/kern/lockdown.c
create mode 100644 include/grub/lockdown.h
diff --git a/conf/Makefile.common b/conf/Makefile.common
index 6cd71cbb2..2a1a886f6 100644
--- a/conf/Makefile.common
+++ b/conf/Makefile.common
@@ -84,7 +84,9 @@ CPPFLAGS_PARTTOOL_LIST =
-Dgrub_parttool_register=PARTTOOL_LIST_MARKER
CPPFLAGS_TERMINAL_LIST =
'-Dgrub_term_register_input(...)=INPUT_TERMINAL_LIST_MARKER(__VA_ARGS__)'
CPPFLAGS_TERMINAL_LIST +=
'-Dgrub_term_register_output(...)=OUTPUT_TERMINAL_LIST_MARKER(__VA_ARGS__)'
CPPFLAGS_COMMAND_LIST =
'-Dgrub_register_command(...)=COMMAND_LIST_MARKER(__VA_ARGS__)'
+CPPFLAGS_COMMAND_LIST +=
'-Dgrub_register_command_lockdown(...)=COMMAND_LOCKDOWN_LIST_MARKER(__VA_ARGS__)'
CPPFLAGS_COMMAND_LIST +=
'-Dgrub_register_extcmd(...)=EXTCOMMAND_LIST_MARKER(__VA_ARGS__)'
+CPPFLAGS_COMMAND_LIST +=
'-Dgrub_register_extcmd_lockdown(...)=EXTCOMMAND_LOCKDOWN_LIST_MARKER(__VA_ARGS__)'
CPPFLAGS_COMMAND_LIST +=
'-Dgrub_register_command_p1(...)=P1COMMAND_LIST_MARKER(__VA_ARGS__)'
CPPFLAGS_FDT_LIST :=
'-Dgrub_fdtbus_register(...)=FDT_DRIVER_LIST_MARKER(__VA_ARGS__)'
CPPFLAGS_MARKER = $(CPPFLAGS_FS_LIST) $(CPPFLAGS_VIDEO_LIST) \
diff --git a/docs/grub-dev.texi b/docs/grub-dev.texi
index 24d17b8ec..a834b3a9c 100644
--- a/docs/grub-dev.texi
+++ b/docs/grub-dev.texi
@@ -86,6 +86,7 @@ This edition documents version @value{VERSION}.
* PFF2 Font File Format::
* Graphical Menu Software Design::
* Verifiers framework::
+* Lockdown framework::
* Copying This Manual:: Copying This Manual
* Index::
@end menu
@@ -2123,6 +2124,32 @@ Optionally at the end of the file @samp{fini}, if it
exists, is called with just
the context. If you return no error during any of @samp{init}, @samp{write} and
@samp{fini} then the file is considered as having succeded verification.
+@node Lockdown framework
+@chapter Lockdown framework
+
+The GRUB can be locked down, which is a restricted mode where some operations
+are not allowed. For instance, some commands cannot be used when the GRUB is
+locked down.
+
+The function
+@code{grub_lockdown()} is used to lockdown GRUB and the function
+@code{grub_is_lockdown()} function can be used to check whether lockdown is
+enabled or not. When enabled, the function returns @samp{GRUB_LOCKDOWN_ENABLED}
+and @samp{GRUB_LOCKDOWN_DISABLED} when is not enabled.
+
+The following functions can be used to register the commands that can only be
+used when lockdown is disabled:
+
+@itemize
+
+@item @code{grub_cmd_lockdown()} registers command which should not run when
the
+GRUB is in lockdown mode.
+
+@item @code{grub_cmd_lockdown()} registers extended command which should not
run
+when the GRUB is in lockdown mode.
+
+@end itemize
+
@node Copying This Manual
@appendix Copying This Manual
diff --git a/docs/grub.texi b/docs/grub.texi
index e3de2a4dc..fd3c78054 100644
---
[SECURITY PATCH 090/117] disk/lvm: Do not overread metadata
From: Daniel Axtens
We could reach the end of valid metadata and not realize, leading to
some buffer overreads. Check if we have reached the end and bail.
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/disk/lvm.c | 35 +--
1 file changed, 29 insertions(+), 6 deletions(-)
diff --git a/grub-core/disk/lvm.c b/grub-core/disk/lvm.c
index 201097fda..7efbaec79 100644
--- a/grub-core/disk/lvm.c
+++ b/grub-core/disk/lvm.c
@@ -352,17 +352,23 @@ grub_lvm_detect (grub_disk_t disk,
while (1)
{
grub_ssize_t s;
- while (grub_isspace (*p))
+ while (grub_isspace (*p) && p < mda_end)
p++;
+ if (p == mda_end)
+ goto fail4;
+
if (*p == '}')
break;
pv = grub_zalloc (sizeof (*pv));
q = p;
- while (*q != ' ')
+ while (*q != ' ' && q < mda_end)
q++;
+ if (q == mda_end)
+ goto pvs_fail_noname;
+
s = q - p;
pv->name = grub_malloc (s + 1);
grub_memcpy (pv->name, p, s);
@@ -405,6 +411,7 @@ grub_lvm_detect (grub_disk_t disk,
continue;
pvs_fail:
grub_free (pv->name);
+ pvs_fail_noname:
grub_free (pv);
goto fail4;
}
@@ -426,18 +433,24 @@ grub_lvm_detect (grub_disk_t disk,
struct grub_diskfilter_segment *seg;
int is_pvmove;
- while (grub_isspace (*p))
+ while (grub_isspace (*p) && p < mda_end)
p++;
+ if (p == mda_end)
+ goto fail4;
+
if (*p == '}')
break;
lv = grub_zalloc (sizeof (*lv));
q = p;
- while (*q != ' ')
+ while (*q != ' ' && q < mda_end)
q++;
+ if (q == mda_end)
+ goto lvs_fail;
+
s = q - p;
lv->name = grub_strndup (p, s);
if (!lv->name)
@@ -609,9 +622,12 @@ grub_lvm_detect (grub_disk_t disk,
if (p == NULL)
goto lvs_segment_fail2;
q = ++p;
- while (*q != '"')
+ while (q < mda_end && *q != '"')
q++;
+ if (q == mda_end)
+ goto lvs_segment_fail2;
+
s = q - p;
stripe->name = grub_malloc (s + 1);
@@ -668,9 +684,12 @@ grub_lvm_detect (grub_disk_t disk,
if (p == NULL)
goto lvs_segment_fail2;
q = ++p;
- while (*q != '"')
+ while (q < mda_end && *q != '"')
q++;
+ if (q == mda_end)
+ goto lvs_segment_fail2;
+
s = q - p;
lvname = grub_malloc (s + 1);
@@ -826,6 +845,8 @@ grub_lvm_detect (grub_disk_t disk,
goto cache_lv_fail;
p3 = ++p2;
+ if (p3 == mda_end)
+ goto cache_lv_fail;
p3 = grub_strchr (p3, '"');
if (!p3)
goto cache_lv_fail;
@@ -847,6 +868,8 @@ grub_lvm_detect (grub_disk_t disk,
goto cache_lv_fail;
p3 = ++p2;
+ if (p3 == mda_end)
+ goto cache_lv_fail;
p3 = grub_strchr (p3, '"');
if (!p3)
goto cache_lv_fail;
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 057/117] util/grub-install: Fix NULL pointer dereferences
Two grub_device_open() calls does not have associated NULL checks
for returned values. Fix that and appease the Coverity.
Fixes: CID 314583
Signed-off-by: Daniel Kiper
Reviewed-by: Javier Martinez Canillas
---
util/grub-install.c | 4
1 file changed, 4 insertions(+)
diff --git a/util/grub-install.c b/util/grub-install.c
index 79fc4b937..a0babe3ef 100644
--- a/util/grub-install.c
+++ b/util/grub-install.c
@@ -1784,6 +1784,8 @@ main (int argc, char *argv[])
fill_core_services (core_services);
ins_dev = grub_device_open (install_drive);
+ if (ins_dev == NULL)
+ grub_util_error ("%s", grub_errmsg);
bless (ins_dev, core_services, 0);
@@ -1884,6 +1886,8 @@ main (int argc, char *argv[])
fill_core_services(core_services);
ins_dev = grub_device_open (install_drive);
+ if (ins_dev == NULL)
+ grub_util_error ("%s", grub_errmsg);
bless (ins_dev, boot_efi, 1);
if (!removable && update_nvram)
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 010/117] commands/setpci: Restrict setpci command when locked down
From: Javier Martinez Canillas
This command can set PCI devices register values, which makes it dangerous
in a locked down configuration. Restrict it so can't be used on this setup.
Signed-off-by: Javier Martinez Canillas
Reviewed-by: Daniel Kiper
---
grub-core/commands/setpci.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/grub-core/commands/setpci.c b/grub-core/commands/setpci.c
index e966af080..8a0c91f02 100644
--- a/grub-core/commands/setpci.c
+++ b/grub-core/commands/setpci.c
@@ -328,10 +328,10 @@ static grub_extcmd_t cmd;
GRUB_MOD_INIT(setpci)
{
- cmd = grub_register_extcmd ("setpci", grub_cmd_setpci, 0,
- N_("[-s POSITION] [-d DEVICE] [-v VAR] "
-"REGISTER[=VALUE[:MASK]]"),
- N_("Manipulate PCI devices."), options);
+ cmd = grub_register_extcmd_lockdown ("setpci", grub_cmd_setpci, 0,
+ N_("[-s POSITION] [-d DEVICE] [-v VAR] "
+ "REGISTER[=VALUE[:MASK]]"),
+ N_("Manipulate PCI devices."), options);
}
GRUB_MOD_FINI(setpci)
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 107/117] util/mkimage: Improve data_size value calculation
From: Peter Jones According to "Microsoft Portable Executable and Common Object File Format Specification", the Optional Header SizeOfInitializedData field contains: Size of the initialized data section, or the sum of all such sections if there are multiple data sections. Make this explicit by adding the GRUB kernel data size to the sum of all the modules sizes. The ALIGN_UP() is not required by the PE spec but do it to avoid alignment issues. Signed-off-by: Peter Jones Signed-off-by: Javier Martinez Canillas Reviewed-by: Daniel Kiper --- util/mkimage.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/util/mkimage.c b/util/mkimage.c index deaef5666..853a52179 100644 --- a/util/mkimage.c +++ b/util/mkimage.c @@ -1260,6 +1260,7 @@ grub_install_generate_image (const char *dir, const char *prefix, void *pe_img; grub_uint8_t *header; void *sections; + size_t scn_size; size_t pe_size; struct grub_pe32_coff_header *c; struct grub_pe32_section_table *text_section, *data_section; @@ -1362,7 +1363,10 @@ grub_install_generate_image (const char *dir, const char *prefix, | GRUB_PE32_SCN_MEM_EXECUTE | GRUB_PE32_SCN_MEM_READ); - PE_OHDR (o32, o64, data_size) = grub_host_to_target32 (reloc_addr - layout.exec_size - header_size); + scn_size = ALIGN_UP (layout.kernel_size - layout.exec_size, GRUB_PE32_FILE_ALIGNMENT); + PE_OHDR (o32, o64, data_size) = grub_host_to_target32 (scn_size + + ALIGN_UP (total_module_size, + GRUB_PE32_FILE_ALIGNMENT)); data_section = text_section + 1; strcpy (data_section->name, ".data"); -- 2.11.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 058/117] util/grub-editenv: Fix incorrect casting of a signed value
From: Darren Kenny
The return value of ftell() may be negative (-1) on error. While it is
probably unlikely to occur, we should not blindly cast to an unsigned
value without first testing that it is not negative.
Fixes: CID 73856
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
util/grub-editenv.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/util/grub-editenv.c b/util/grub-editenv.c
index f3662c95b..db6f187cc 100644
--- a/util/grub-editenv.c
+++ b/util/grub-editenv.c
@@ -125,6 +125,7 @@ open_envblk_file (const char *name)
{
FILE *fp;
char *buf;
+ long loc;
size_t size;
grub_envblk_t envblk;
@@ -143,7 +144,12 @@ open_envblk_file (const char *name)
grub_util_error (_("cannot seek `%s': %s"), name,
strerror (errno));
- size = (size_t) ftell (fp);
+ loc = ftell (fp);
+ if (loc < 0)
+grub_util_error (_("cannot get file location `%s': %s"), name,
+strerror (errno));
+
+ size = (size_t) loc;
if (fseek (fp, 0, SEEK_SET) < 0)
grub_util_error (_("cannot seek `%s': %s"), name,
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 012/117] gdb: Restrict GDB access when locked down
From: Javier Martinez Canillas
The gdbstub* commands allow to start and control a GDB stub running on
local host that can be used to connect from a remote debugger. Restrict
this functionality when the GRUB is locked down.
Signed-off-by: Javier Martinez Canillas
Reviewed-by: Daniel Kiper
---
grub-core/gdb/gdb.c | 32 ++--
1 file changed, 18 insertions(+), 14 deletions(-)
diff --git a/grub-core/gdb/gdb.c b/grub-core/gdb/gdb.c
index 847a1e1e3..1818cb6f8 100644
--- a/grub-core/gdb/gdb.c
+++ b/grub-core/gdb/gdb.c
@@ -75,20 +75,24 @@ static grub_command_t cmd, cmd_stop, cmd_break;
GRUB_MOD_INIT (gdb)
{
grub_gdb_idtinit ();
- cmd = grub_register_command ("gdbstub", grub_cmd_gdbstub,
- N_("PORT"),
- /* TRANSLATORS: GDB stub is a small part of
- GDB functionality running on local host
- which allows remote debugger to
- connect to it. */
- N_("Start GDB stub on given port"));
- cmd_break = grub_register_command ("gdbstub_break", grub_cmd_gdb_break,
-/* TRANSLATORS: this refers to triggering
- a breakpoint so that the user will land
- into GDB. */
-0, N_("Break into GDB"));
- cmd_stop = grub_register_command ("gdbstub_stop", grub_cmd_gdbstop,
- 0, N_("Stop GDB stub"));
+ cmd = grub_register_command_lockdown ("gdbstub", grub_cmd_gdbstub,
+ N_("PORT"),
+ /*
+* TRANSLATORS: GDB stub is a small
part of
+* GDB functionality running on local
host
+* which allows remote debugger to
+* connect to it.
+*/
+ N_("Start GDB stub on given port"));
+ cmd_break = grub_register_command_lockdown ("gdbstub_break",
grub_cmd_gdb_break,
+ /*
+ * TRANSLATORS: this refers to
triggering
+ * a breakpoint so that the user
will land
+ * into GDB.
+ */
+ 0, N_("Break into GDB"));
+ cmd_stop = grub_register_command_lockdown ("gdbstub_stop", grub_cmd_gdbstop,
+0, N_("Stop GDB stub"));
}
GRUB_MOD_FINI (gdb)
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 115/117] gfxmenu/gui: Check printf() format in the gui_progress_bar and gui_label
From: Thomas Frauendorfer | Miray Software
The gui_progress_bar and gui_label components can display the timeout
value. The format string can be set through a theme file. This patch
adds a validation step to the format string.
If a user loads a theme file into the GRUB without this patch then
a GUI label with the following settings
+ label {
...
id = "__timeout__"
text = "%s"
}
will interpret the current timeout value as string pointer and print the
memory at that position on the screen. It is not desired behavior.
Signed-off-by: Thomas Frauendorfer | Miray Software
Reviewed-by: Daniel Kiper
---
grub-core/gfxmenu/gui_label.c| 4
grub-core/gfxmenu/gui_progress_bar.c | 3 +++
2 files changed, 7 insertions(+)
diff --git a/grub-core/gfxmenu/gui_label.c b/grub-core/gfxmenu/gui_label.c
index a4c817891..1c190542a 100644
--- a/grub-core/gfxmenu/gui_label.c
+++ b/grub-core/gfxmenu/gui_label.c
@@ -193,6 +193,10 @@ label_set_property (void *vself, const char *name, const
char *value)
else if (grub_strcmp (value, "@KEYMAP_SHORT@") == 0)
value = _("enter: boot, `e': options, `c': cmd-line");
/* FIXME: Add more templates here if needed. */
+
+ if (grub_printf_fmt_check(value, "%d") != GRUB_ERR_NONE)
+ value = ""; /* Unsupported format. */
+
self->template = grub_strdup (value);
self->text = grub_xasprintf (value, self->value);
}
diff --git a/grub-core/gfxmenu/gui_progress_bar.c
b/grub-core/gfxmenu/gui_progress_bar.c
index b128f0866..ace85a125 100644
--- a/grub-core/gfxmenu/gui_progress_bar.c
+++ b/grub-core/gfxmenu/gui_progress_bar.c
@@ -348,6 +348,9 @@ progress_bar_set_property (void *vself, const char *name,
const char *value)
Please use the shortest form available in you language. */
value = _("%ds");
+ if (grub_printf_fmt_check(value, "%d") != GRUB_ERR_NONE)
+ value = ""; /* Unsupported format. */
+
self->template = grub_strdup (value);
}
else if (grub_strcmp (name, "font") == 0)
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 062/117] script/execute: Avoid crash when using "$#" outside a function scope
From: Daniel Axtens
"$#" represents the number of arguments to a function. It is only
defined in a function scope, where "scope" is non-NULL. Currently,
if we attempt to evaluate "$#" outside a function scope, "scope" will
be NULL and we will crash with a NULL pointer dereference.
Do not attempt to count arguments for "$#" if "scope" is NULL. This
will result in "$#" being interpreted as an empty string if evaluated
outside a function scope.
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/script/execute.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/grub-core/script/execute.c b/grub-core/script/execute.c
index 3ad468fce..b88765f1d 100644
--- a/grub-core/script/execute.c
+++ b/grub-core/script/execute.c
@@ -485,7 +485,7 @@ gettext_putvar (const char *str, grub_size_t len,
return 0;
/* Enough for any number. */
- if (len == 1 && str[0] == '#')
+ if (len == 1 && str[0] == '#' && scope != NULL)
{
grub_snprintf (*ptr, 30, "%u", scope->argv.argc);
*ptr += grub_strlen (*ptr);
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 014/117] docs: Document the cutmem command
From: Javier Martinez Canillas
The command is not present in the docs/grub.texi user documentation.
Reported-by: Daniel Kiper
Signed-off-by: Javier Martinez Canillas
Signed-off-by: Daniel Kiper
Reviewed-by: Javier Martinez Canillas
---
docs/grub.texi | 21 +
1 file changed, 21 insertions(+)
diff --git a/docs/grub.texi b/docs/grub.texi
index 91666781b..c5fc3904f 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -3981,6 +3981,7 @@ you forget a command, you can run the command
@command{help}
* cpuid:: Check for CPU features
* crc:: Compute or check CRC32 checksums
* cryptomount:: Mount a crypto device
+* cutmem:: Remove memory regions
* date::Display or set current date and time
* devicetree:: Load a device tree blob
* distrust::Remove a pubkey from trusted keys
@@ -4139,6 +4140,8 @@ this page is to be filtered. This syntax makes it easy
to represent patterns
that are often result of memory damage, due to physical distribution of memory
cells.
+The command is similar to @command{cutmem} command.
+
Note: The command is not allowed when lockdown is enforced (@pxref{Lockdown}).
This prevents removing EFI memory regions to potentially subvert the
security mechanisms provided by the UEFI secure boot.
@@ -4303,6 +4306,24 @@ modules (@var{luks}, @var{luks2} and @var{geli}) have to
be loaded manually
before this command can be used.
@end deffn
+@node cutmem
+@subsection cutmem
+
+@deffn Command cutmem from[K|M|G] to[K|M|G]
+Remove any memory regions in specified range.
+@end deffn
+
+This command notifies the memory manager that specified regions of RAM ought to
+be filtered out. This remains in effect after a payload kernel has been loaded
+by GRUB, as long as the loaded kernel obtains its memory map from GRUB. Kernels
+that support this include Linux, GNU Mach, the kernel of FreeBSD and Multiboot
+kernels in general.
+
+The command is similar to @command{badram} command.
+
+Note: The command is not allowed when lockdown is enforced (@pxref{Lockdown}).
+ This prevents removing EFI memory regions to potentially subvert the
+ security mechanisms provided by the UEFI secure boot.
@node date
@subsection date
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 114/117] kern/misc: Add function to check printf() format against expected format
From: Thomas Frauendorfer | Miray Software
The grub_printf_fmt_check() function parses the arguments of an untrusted
printf() format and an expected printf() format and then compares the
arguments counts and arguments types. The arguments count in the untrusted
format string must be less or equal to the arguments count in the expected
format string and both arguments types must match.
To do this the parse_printf_arg_fmt() helper function is extended in the
following way:
1. Add a return value to report errors to the grub_printf_fmt_check().
2. Add the fmt_check argument to enable stricter format verification:
- the function expects that arguments definitions are always
terminated by a supported conversion specifier.
- positional parameters, "$", are not allowed, as they cannot be
validated correctly with the current implementation. For example
"%s%1$d" would assign the first args entry twice while leaving the
second one unchanged.
- Return an error if preallocated space in args is too small and
allocation fails for the needed size. The grub_printf_fmt_check()
should verify all arguments. So, if validation is not possible for
any reason it should return an error.
This also adds a case entry to handle "%%", which is the escape
sequence to print "%" character.
3. Add the max_args argument to check for the maximum allowed arguments
count in a printf() string. This should be set to the arguments count
of the expected format. Then the parse_printf_arg_fmt() function will
return an error if the arguments count is exceeded.
The two additional arguments allow us to use parse_printf_arg_fmt() in
printf() and grub_printf_fmt_check() calls.
When parse_printf_arg_fmt() is used by grub_printf_fmt_check() the
function parse user provided untrusted format string too. So, in
that case it is better to be too strict than too lenient.
Signed-off-by: Thomas Frauendorfer | Miray Software
Reviewed-by: Daniel Kiper
---
grub-core/kern/misc.c | 82 ---
include/grub/misc.h | 16 ++
2 files changed, 94 insertions(+), 4 deletions(-)
diff --git a/grub-core/kern/misc.c b/grub-core/kern/misc.c
index 074728b2b..3af336ee2 100644
--- a/grub-core/kern/misc.c
+++ b/grub-core/kern/misc.c
@@ -645,8 +645,26 @@ grub_lltoa (char *str, int c, unsigned long long n)
return p;
}
-static void
-parse_printf_arg_fmt (const char *fmt0, struct printf_args *args)
+/*
+ * Parse printf() fmt0 string into args arguments.
+ *
+ * The parsed arguments are either used by a printf() function to format the
fmt0
+ * string or they are used to compare a format string from an untrusted source
+ * against a format string with expected arguments.
+ *
+ * When the fmt_check is set to !0, e.g. 1, then this function is executed in
+ * printf() format check mode. This enforces stricter rules for parsing the
+ * fmt0 to limit exposure to possible errors in printf() handling. It also
+ * disables positional parameters, "$", because some formats, e.g "%s%1$d",
+ * cannot be validated with the current implementation.
+ *
+ * The max_args allows to set a maximum number of accepted arguments. If the
fmt0
+ * string defines more arguments than the max_args then the
parse_printf_arg_fmt()
+ * function returns an error. This is currently used for format check only.
+ */
+static grub_err_t
+parse_printf_arg_fmt (const char *fmt0, struct printf_args *args,
+ int fmt_check, grub_size_t max_args)
{
const char *fmt;
char c;
@@ -673,7 +691,12 @@ parse_printf_arg_fmt (const char *fmt0, struct printf_args
*args)
fmt++;
if (*fmt == '$')
- fmt++;
+ {
+ if (fmt_check)
+ return grub_error (GRUB_ERR_BAD_ARGUMENT,
+ "positional arguments are not supported");
+ fmt++;
+ }
if (*fmt =='-')
fmt++;
@@ -705,9 +728,19 @@ parse_printf_arg_fmt (const char *fmt0, struct printf_args
*args)
case 's':
args->count++;
break;
+ case '%':
+ /* "%%" is the escape sequence to output "%". */
+ break;
+ default:
+ if (fmt_check)
+ return grub_error (GRUB_ERR_BAD_ARGUMENT, "unexpected format");
+ break;
}
}
+ if (fmt_check && args->count > max_args)
+return grub_error (GRUB_ERR_BAD_ARGUMENT, "too many arguments");
+
if (args->count <= ARRAY_SIZE (args->prealloc))
args->ptr = args->prealloc;
else
@@ -715,6 +748,9 @@ parse_printf_arg_fmt (const char *fmt0, struct printf_args
*args)
args->ptr = grub_calloc (args->count, sizeof (args->ptr[0]));
if (!args->ptr)
{
+ if (fmt_check)
+ return grub_errno;
+
grub_errno = GRUB_ERR_NONE;
args->ptr = args->prealloc;
args->count = ARRAY_SIZE (args->prealloc);
@@ -806,6 +842,8 @@ parse_printf_arg_f
[SECURITY PATCH 067/117] video/readers/jpeg: Catch files with unsupported quantization or Huffman tables
From: Daniel Axtens Our decoder only supports 2 quantization tables. If a file asks for a quantization table with index > 1, reject it. Similarly, our decoder only supports 4 Huffman tables. If a file asks for a Huffman table with index > 3, reject it. This fixes some out of bounds reads. It's not clear what degree of control over subsequent execution could be gained by someone who can carefully set up the contents of memory before loading an invalid JPEG file. Signed-off-by: Daniel Axtens Reviewed-by: Daniel Kiper --- grub-core/video/readers/jpeg.c | 8 1 file changed, 8 insertions(+) diff --git a/grub-core/video/readers/jpeg.c b/grub-core/video/readers/jpeg.c index 0b6ce3cee..23f919aa0 100644 --- a/grub-core/video/readers/jpeg.c +++ b/grub-core/video/readers/jpeg.c @@ -333,7 +333,11 @@ grub_jpeg_decode_sof (struct grub_jpeg_data *data) else if (ss != JPEG_SAMPLING_1x1) return grub_error (GRUB_ERR_BAD_FILE_TYPE, "jpeg: sampling method not supported"); + data->comp_index[id][0] = grub_jpeg_get_byte (data); + if (data->comp_index[id][0] > 1) + return grub_error (GRUB_ERR_BAD_FILE_TYPE, + "jpeg: too many quantization tables"); } if (data->file->offset != next_marker) @@ -602,6 +606,10 @@ grub_jpeg_decode_sos (struct grub_jpeg_data *data) ht = grub_jpeg_get_byte (data); data->comp_index[id][1] = (ht >> 4); data->comp_index[id][2] = (ht & 0xF) + 2; + + if ((data->comp_index[id][1] < 0) || (data->comp_index[id][1] > 3) || + (data->comp_index[id][2] < 0) || (data->comp_index[id][2] > 3)) + return grub_error (GRUB_ERR_BAD_FILE_TYPE, "jpeg: invalid hufftable index"); } grub_jpeg_get_byte (data); /* Skip 3 unused bytes. */ -- 2.11.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 021/117] kern/efi: Fix memory leak on failure
From: Darren Kenny
Free the memory allocated to name before returning on failure.
Fixes: CID 296222
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
grub-core/kern/efi/efi.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/grub-core/kern/efi/efi.c b/grub-core/kern/efi/efi.c
index 2942b8e35..67394bb6f 100644
--- a/grub-core/kern/efi/efi.c
+++ b/grub-core/kern/efi/efi.c
@@ -410,6 +410,7 @@ grub_efi_get_filename (grub_efi_device_path_t *dp0)
{
grub_error (GRUB_ERR_OUT_OF_RANGE,
"malformed EFI Device Path node has length=%d", len);
+ grub_free (name);
return NULL;
}
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 117/117] kern/mm: Fix grub_debug_calloc() compilation error
From: Marco A Benatto
Fix compilation error due to missing parameter to
grub_printf() when MM_DEBUG is defined.
Fixes: 64e26162e (calloc: Make sure we always have an overflow-checking
calloc() available)
Signed-off-by: Marco A Benatto
Reviewed-by: Daniel Kiper
---
grub-core/kern/mm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/grub-core/kern/mm.c b/grub-core/kern/mm.c
index f2822a836..c070afc62 100644
--- a/grub-core/kern/mm.c
+++ b/grub-core/kern/mm.c
@@ -594,7 +594,7 @@ grub_debug_calloc (const char *file, int line, grub_size_t
nmemb, grub_size_t si
if (grub_mm_debug)
grub_printf ("%s:%d: calloc (0x%" PRIxGRUB_SIZE ", 0x%" PRIxGRUB_SIZE ") =
",
-file, line, size);
+file, line, nmemb, size);
ptr = grub_calloc (nmemb, size);
if (grub_mm_debug)
grub_printf ("%p\n", ptr);
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 071/117] fs/fshelp: Catch impermissibly large block sizes in read helper
From: Daniel Axtens
A fuzzed HFS+ filesystem had log2blocksize = 22. This gave
log2blocksize + GRUB_DISK_SECTOR_BITS = 31. 1 << 31 = 0x8000,
which is -1 as an int. This caused some wacky behavior later on in
the function, leading to out-of-bounds writes on the destination buffer.
Catch log2blocksize + GRUB_DISK_SECTOR_BITS >= 31. We could be stricter,
but this is the minimum that will prevent integer size weirdness.
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/fs/fshelp.c | 12
1 file changed, 12 insertions(+)
diff --git a/grub-core/fs/fshelp.c b/grub-core/fs/fshelp.c
index 4c902adf3..a2d0d297a 100644
--- a/grub-core/fs/fshelp.c
+++ b/grub-core/fs/fshelp.c
@@ -362,6 +362,18 @@ grub_fshelp_read_file (grub_disk_t disk,
grub_fshelp_node_t node,
grub_disk_addr_t i, blockcnt;
int blocksize = 1 << (log2blocksize + GRUB_DISK_SECTOR_BITS);
+ /*
+ * Catch blatantly invalid log2blocksize. We could be a lot stricter, but
+ * this is the most permissive we can be before we start to see integer
+ * overflow/underflow issues.
+ */
+ if (log2blocksize + GRUB_DISK_SECTOR_BITS >= 31)
+{
+ grub_error (GRUB_ERR_OUT_OF_RANGE,
+ N_("blocksize too large"));
+ return -1;
+}
+
if (pos > filesize)
{
grub_error (GRUB_ERR_OUT_OF_RANGE,
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 019/117] net/tftp: Fix dangling memory pointer
From: Darren Kenny The static code analysis tool, Parfait, reported that the valid of file->data was left referencing memory that was freed by the call to grub_free(data) where data was initialized from file->data. To ensure that there is no unintentional access to this memory referenced by file->data we should set the pointer to NULL. Signed-off-by: Darren Kenny Reviewed-by: Daniel Kiper --- grub-core/net/tftp.c | 1 + 1 file changed, 1 insertion(+) diff --git a/grub-core/net/tftp.c b/grub-core/net/tftp.c index 7e659a78b..1f3793ed8 100644 --- a/grub-core/net/tftp.c +++ b/grub-core/net/tftp.c @@ -442,6 +442,7 @@ tftp_close (struct grub_file *file) grub_net_udp_close (data->sock); } grub_free (data); + file->data = NULL; return GRUB_ERR_NONE; } -- 2.11.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 113/117] kern/misc: Add STRING type for internal printf() format handling
From: Thomas Frauendorfer | Miray Software
Set printf() argument type for "%s" to new type STRING. This is in
preparation for a follow up patch to compare a printf() format string
against an expected printf() format string.
For "%s" the corresponding printf() argument is dereferenced as pointer
while all other argument types are defined as integer value. However,
when validating a printf() format it is necessary to differentiate "%s"
from "%p" and other integers. So, let's do that.
Signed-off-by: Thomas Frauendorfer | Miray Software
Reviewed-by: Daniel Kiper
---
grub-core/kern/misc.c | 13 +++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/grub-core/kern/misc.c b/grub-core/kern/misc.c
index c58857ca2..074728b2b 100644
--- a/grub-core/kern/misc.c
+++ b/grub-core/kern/misc.c
@@ -33,7 +33,8 @@ union printf_arg
enum
{
INT, LONG, LONGLONG,
- UNSIGNED_INT = 3, UNSIGNED_LONG, UNSIGNED_LONGLONG
+ UNSIGNED_INT = 3, UNSIGNED_LONG, UNSIGNED_LONGLONG,
+ STRING
} type;
long long ll;
};
@@ -791,12 +792,14 @@ parse_printf_arg_fmt (const char *fmt0, struct
printf_args *args)
args->ptr[curn].type = INT + longfmt;
break;
case 'p':
- case 's':
if (sizeof (void *) == sizeof (long long))
args->ptr[curn].type = UNSIGNED_LONGLONG;
else
args->ptr[curn].type = UNSIGNED_INT;
break;
+ case 's':
+ args->ptr[curn].type = STRING;
+ break;
case 'C':
case 'c':
args->ptr[curn].type = INT;
@@ -831,6 +834,12 @@ parse_printf_args (const char *fmt0, struct printf_args
*args, va_list args_in)
case UNSIGNED_LONGLONG:
args->ptr[n].ll = va_arg (args_in, long long);
break;
+ case STRING:
+ if (sizeof (void *) == sizeof (long long))
+ args->ptr[n].ll = va_arg (args_in, long long);
+ else
+ args->ptr[n].ll = va_arg (args_in, unsigned int);
+ break;
}
}
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 082/117] io/gzio: Bail if gzio->tl/td is NULL
From: Daniel Axtens
This is an ugly fix that doesn't address why gzio->tl comes to be NULL.
However, it seems to be sufficient to patch up a bunch of NULL derefs.
It would be good to revisit this in future and see if we can have
a cleaner solution that addresses some of the causes of the unexpected
NULL pointers.
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/io/gzio.c | 20
1 file changed, 20 insertions(+)
diff --git a/grub-core/io/gzio.c b/grub-core/io/gzio.c
index 43d98a7bd..4a8eaeae2 100644
--- a/grub-core/io/gzio.c
+++ b/grub-core/io/gzio.c
@@ -669,6 +669,13 @@ inflate_codes_in_window (grub_gzio_t gzio)
{
if (! gzio->code_state)
{
+
+ if (gzio->tl == NULL)
+ {
+ grub_error (GRUB_ERR_BAD_COMPRESSED_DATA, "NULL gzio->tl");
+ return 1;
+ }
+
NEEDBITS ((unsigned) gzio->bl);
if ((e = (t = gzio->tl + ((unsigned) b & ml))->e) > 16)
do
@@ -707,6 +714,12 @@ inflate_codes_in_window (grub_gzio_t gzio)
n = t->v.n + ((unsigned) b & mask_bits[e]);
DUMPBITS (e);
+ if (gzio->td == NULL)
+ {
+ grub_error (GRUB_ERR_BAD_COMPRESSED_DATA, "NULL gzio->td");
+ return 1;
+ }
+
/* decode distance of block to copy */
NEEDBITS ((unsigned) gzio->bd);
if ((e = (t = gzio->td + ((unsigned) b & md))->e) > 16)
@@ -917,6 +930,13 @@ init_dynamic_block (grub_gzio_t gzio)
n = nl + nd;
m = mask_bits[gzio->bl];
i = l = 0;
+
+ if (gzio->tl == NULL)
+{
+ grub_error (GRUB_ERR_BAD_COMPRESSED_DATA, "NULL gzio->tl");
+ return;
+}
+
while ((unsigned) i < n)
{
NEEDBITS ((unsigned) gzio->bl);
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 078/117] fs/jfs: Catch infinite recursion
From: Daniel Axtens
It's possible with a fuzzed filesystem for JFS to keep getblk()-ing
the same data over and over again, leading to stack exhaustion.
Check if we'd be calling the function with exactly the same data as
was passed in, and if so abort.
I'm not sure what the performance impact of this is and am open to
better ideas.
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/fs/jfs.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/grub-core/fs/jfs.c b/grub-core/fs/jfs.c
index 804c42d31..6f7c43904 100644
--- a/grub-core/fs/jfs.c
+++ b/grub-core/fs/jfs.c
@@ -304,7 +304,16 @@ getblk (struct grub_jfs_treehead *treehead,
<< (grub_le_to_cpu16 (data->sblock.log2_blksz)
- GRUB_DISK_SECTOR_BITS), 0,
sizeof (*tree), (char *) tree))
- ret = getblk (&tree->treehead, &tree->extents[0], 254, data, blk);
+ {
+ if (grub_memcmp (&tree->treehead, treehead, sizeof (struct
grub_jfs_treehead)) ||
+ grub_memcmp (&tree->extents, extents, 254 * sizeof (struct
grub_jfs_tree_extent)))
+ ret = getblk (&tree->treehead, &tree->extents[0], 254, data, blk);
+ else
+ {
+ grub_error (GRUB_ERR_BAD_FS, "jfs: infinite recursion detected");
+ ret = -1;
+ }
+ }
grub_free (tree);
return ret;
}
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 034/117] disk/cryptodisk: Fix potential integer overflow
From: Darren Kenny The encrypt and decrypt functions expect a grub_size_t. So, we need to ensure that the constant bit shift is using grub_size_t rather than unsigned int when it is performing the shift. Fixes: CID 307788 Signed-off-by: Darren Kenny Reviewed-by: Daniel Kiper --- grub-core/disk/cryptodisk.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c index b62835acc..41866c62d 100644 --- a/grub-core/disk/cryptodisk.c +++ b/grub-core/disk/cryptodisk.c @@ -325,10 +325,10 @@ grub_cryptodisk_endecrypt (struct grub_cryptodisk *dev, case GRUB_CRYPTODISK_MODE_CBC: if (do_encrypt) err = grub_crypto_cbc_encrypt (dev->cipher, data + i, data + i, - (1U << log_sector_size), iv); + ((grub_size_t) 1 << log_sector_size), iv); else err = grub_crypto_cbc_decrypt (dev->cipher, data + i, data + i, - (1U << log_sector_size), iv); + ((grub_size_t) 1 << log_sector_size), iv); if (err) return err; break; @@ -336,10 +336,10 @@ grub_cryptodisk_endecrypt (struct grub_cryptodisk *dev, case GRUB_CRYPTODISK_MODE_PCBC: if (do_encrypt) err = grub_crypto_pcbc_encrypt (dev->cipher, data + i, data + i, - (1U << log_sector_size), iv); + ((grub_size_t) 1 << log_sector_size), iv); else err = grub_crypto_pcbc_decrypt (dev->cipher, data + i, data + i, - (1U << log_sector_size), iv); + ((grub_size_t) 1 << log_sector_size), iv); if (err) return err; break; -- 2.11.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 084/117] io/gzio: Catch missing values in huft_build() and bail
From: Daniel Axtens
In huft_build(), "v" is a table of values in order of bit length.
The code later (when setting up table entries in "r") assumes that all
elements of this array corresponding to a code are initialized and less
than N_MAX. However, it doesn't enforce this.
With sufficiently manipulated inputs (e.g. from fuzzing), there can be
elements of "v" that are not filled. Therefore a lookup into "e" or "d"
will use an uninitialized value. This can lead to an invalid/OOB read on
those values, often leading to a crash.
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/io/gzio.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/grub-core/io/gzio.c b/grub-core/io/gzio.c
index 4236f0fd4..19adebeed 100644
--- a/grub-core/io/gzio.c
+++ b/grub-core/io/gzio.c
@@ -507,6 +507,7 @@ huft_build (unsigned *b,/* code lengths in bits (all
assumed <= BMAX) */
}
/* Make a table of values in order of bit lengths */
+ grub_memset (v, N_MAX, ARRAY_SIZE (v));
p = b;
i = 0;
do
@@ -588,11 +589,18 @@ huft_build (unsigned *b, /* code lengths in bits (all
assumed <= BMAX) */
r.v.n = (ush) (*p); /* simple code is just the value */
p++; /* one compiler does not like *p++ */
}
- else
+ else if (*p < N_MAX)
{
r.e = (uch) e[*p - s];/* non-simple--look up in lists */
r.v.n = d[*p++ - s];
}
+ else
+ {
+ /* Detected an uninitialised value, abort. */
+ if (h)
+ huft_free (u[0]);
+ return 2;
+ }
/* fill code-like entries with r */
f = 1 << (k - w);
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 037/117] zfs: Fix resource leaks while constructing path
From: Paulo Flabiano Smorigo
There are several exit points in dnode_get_path() that are causing possible
memory leaks.
In the while(1) the correct exit mechanism should not be to do a direct return,
but to instead break out of the loop, setting err first if it is not already
set.
The reason behind this is that the dnode_path is a linked list, and while doing
through this loop, it is being allocated and built up - the only way to
correctly unravel it is to traverse it, which is what is being done at the end
of the function outside of the loop.
Several of the existing exit points correctly did a break, but not all so this
change makes that more consistent and should resolve the leaking of memory as
found by Coverity.
Fixes: CID 73741
Signed-off-by: Paulo Flabiano Smorigo
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
grub-core/fs/zfs/zfs.c | 30 +-
1 file changed, 21 insertions(+), 9 deletions(-)
diff --git a/grub-core/fs/zfs/zfs.c b/grub-core/fs/zfs/zfs.c
index 695e6ea30..ca82eb878 100644
--- a/grub-core/fs/zfs/zfs.c
+++ b/grub-core/fs/zfs/zfs.c
@@ -2839,8 +2839,8 @@ dnode_get_path (struct subvolume *subvol, const char
*path_in, dnode_end_t *dn,
if (dnode_path->dn.dn.dn_type != DMU_OT_DIRECTORY_CONTENTS)
{
- grub_free (path_buf);
- return grub_error (GRUB_ERR_BAD_FILE_TYPE, N_("not a directory"));
+ err = grub_error (GRUB_ERR_BAD_FILE_TYPE, N_("not a directory"));
+ break;
}
err = zap_lookup (&(dnode_path->dn), cname, &objnum,
data, subvol->case_insensitive);
@@ -2882,11 +2882,18 @@ dnode_get_path (struct subvolume *subvol, const char
*path_in, dnode_end_t *dn,
<< SPA_MINBLOCKSHIFT);
if (blksz == 0)
- return grub_error(GRUB_ERR_BAD_FS, "0-sized block");
+{
+ err = grub_error (GRUB_ERR_BAD_FS, "0-sized block");
+ break;
+}
sym_value = grub_malloc (sym_sz);
if (!sym_value)
- return grub_errno;
+ {
+ err = grub_errno;
+ break;
+ }
+
for (block = 0; block < (sym_sz + blksz - 1) / blksz; block++)
{
void *t;
@@ -2896,7 +2903,7 @@ dnode_get_path (struct subvolume *subvol, const char
*path_in, dnode_end_t *dn,
if (err)
{
grub_free (sym_value);
- return err;
+ break;
}
movesize = sym_sz - block * blksz;
@@ -2906,6 +2913,8 @@ dnode_get_path (struct subvolume *subvol, const char
*path_in, dnode_end_t *dn,
grub_memcpy (sym_value + block * blksz, t, movesize);
grub_free (t);
}
+ if (err)
+ break;
free_symval = 1;
}
path = path_buf = grub_malloc (sym_sz + grub_strlen (oldpath) + 1);
@@ -2914,7 +2923,8 @@ dnode_get_path (struct subvolume *subvol, const char
*path_in, dnode_end_t *dn,
grub_free (oldpathbuf);
if (free_symval)
grub_free (sym_value);
- return grub_errno;
+ err = grub_errno;
+ break;
}
grub_memcpy (path, sym_value, sym_sz);
if (free_symval)
@@ -2952,11 +2962,12 @@ dnode_get_path (struct subvolume *subvol, const char
*path_in, dnode_end_t *dn,
err = zio_read (bp, dnode_path->dn.endian, &sahdrp, NULL, data);
if (err)
- return err;
+ break;
}
else
{
- return grub_error (GRUB_ERR_BAD_FS, "filesystem is corrupt");
+ err = grub_error (GRUB_ERR_BAD_FS, "filesystem is corrupt");
+ break;
}
hdrsize = SA_HDR_SIZE (((sa_hdr_phys_t *) sahdrp));
@@ -2977,7 +2988,8 @@ dnode_get_path (struct subvolume *subvol, const char
*path_in, dnode_end_t *dn,
if (!path_buf)
{
grub_free (oldpathbuf);
- return grub_errno;
+ err = grub_errno;
+ break;
}
grub_memcpy (path, sym_value, sym_sz);
path [sym_sz] = 0;
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 086/117] disk/lvm: Don't go beyond the end of the data we read from disk
From: Daniel Axtens
We unconditionally trusted offset_xl from the LVM label header, even if
it told us that the PV header/disk locations were way off past the end
of the data we read from disk.
Require that the offset be sane, fixing an OOB read and crash.
Fixes: CID 314367, CID 314371
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/disk/lvm.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/grub-core/disk/lvm.c b/grub-core/disk/lvm.c
index 753545146..6988fe492 100644
--- a/grub-core/disk/lvm.c
+++ b/grub-core/disk/lvm.c
@@ -178,6 +178,20 @@ grub_lvm_detect (grub_disk_t disk,
goto fail;
}
+ /*
+ * We read a grub_lvm_pv_header and then 2 grub_lvm_disk_locns that
+ * immediately follow the PV header. Make sure we have space for both.
+ */
+ if (grub_le_to_cpu32 (lh->offset_xl) >=
+ GRUB_LVM_LABEL_SIZE - sizeof (struct grub_lvm_pv_header) -
+ 2 * sizeof (struct grub_lvm_disk_locn))
+{
+#ifdef GRUB_UTIL
+ grub_util_info ("LVM PV header/disk locations are beyond the end of the
block");
+#endif
+ goto fail;
+}
+
pvh = (struct grub_lvm_pv_header *) (buf + grub_le_to_cpu32(lh->offset_xl));
for (i = 0, j = 0; i < GRUB_LVM_ID_LEN; i++)
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 044/117] normal/completion: Fix leaking of memory when processing a completion
From: Darren Kenny
It is possible for the code to reach the end of the function without
freeing the memory allocated to argv and argc still to be 0.
We should always call grub_free(argv). The grub_free() will handle
a NULL argument correctly if it reaches that code without the memory
being allocated.
Fixes: CID 96672
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
grub-core/normal/completion.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/grub-core/normal/completion.c b/grub-core/normal/completion.c
index c07100a8d..18cadfa85 100644
--- a/grub-core/normal/completion.c
+++ b/grub-core/normal/completion.c
@@ -401,8 +401,8 @@ char *
grub_normal_do_completion (char *buf, int *restore,
void (*hook) (const char *, grub_completion_type_t,
int))
{
- int argc;
- char **argv;
+ int argc = 0;
+ char **argv = NULL;
/* Initialize variables. */
match = 0;
@@ -517,10 +517,8 @@ grub_normal_do_completion (char *buf, int *restore,
fail:
if (argc != 0)
-{
- grub_free (argv[0]);
- grub_free (argv);
-}
+grub_free (argv[0]);
+ grub_free (argv);
grub_free (match);
grub_errno = GRUB_ERR_NONE;
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 088/117] disk/lvm: Bail on missing PV list
From: Daniel Axtens
There's an if block for the presence of "physical_volumes {", but if
that block is absent, then p remains NULL and a NULL-deref will result
when looking for logical volumes.
It doesn't seem like LVM makes sense without physical volumes, so error
out rather than crashing.
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/disk/lvm.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/grub-core/disk/lvm.c b/grub-core/disk/lvm.c
index b70d7d79f..31bbc9acc 100644
--- a/grub-core/disk/lvm.c
+++ b/grub-core/disk/lvm.c
@@ -409,6 +409,8 @@ grub_lvm_detect (grub_disk_t disk,
goto fail4;
}
}
+ else
+goto fail4;
p = grub_strstr (p, "logical_volumes {");
if (p)
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 042/117] libgcrypt/mpi: Fix possible NULL dereference
From: Darren Kenny The code in gcry_mpi_scan() assumes that buffer is not NULL, but there is no explicit check for that, so we add one. Fixes: CID 73757 Signed-off-by: Darren Kenny Reviewed-by: Daniel Kiper --- grub-core/lib/libgcrypt/mpi/mpicoder.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/grub-core/lib/libgcrypt/mpi/mpicoder.c b/grub-core/lib/libgcrypt/mpi/mpicoder.c index 7ecad27b2..6fe389165 100644 --- a/grub-core/lib/libgcrypt/mpi/mpicoder.c +++ b/grub-core/lib/libgcrypt/mpi/mpicoder.c @@ -379,6 +379,9 @@ gcry_mpi_scan (struct gcry_mpi **ret_mpi, enum gcry_mpi_format format, unsigned int len; int secure = (buffer && gcry_is_secure (buffer)); + if (!buffer) +return gcry_error (GPG_ERR_INV_ARG); + if (format == GCRYMPI_FMT_SSH) len = 0; else -- 2.11.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 093/117] fs/btrfs: Validate the number of stripes/parities in RAID5/6
From: Daniel Axtens This prevents a divide by zero if nstripes == nparities, and also prevents propagation of invalid values if nstripes ends up less than nparities. Signed-off-by: Daniel Axtens Reviewed-by: Daniel Kiper --- grub-core/fs/btrfs.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c index 27339bdb3..c4ba5f110 100644 --- a/grub-core/fs/btrfs.c +++ b/grub-core/fs/btrfs.c @@ -1083,6 +1083,9 @@ grub_btrfs_read_logical (struct grub_btrfs_data *data, grub_disk_addr_t addr, * stripen is computed without the parities * (0 for A0, A1, A2, 1 for B0, B1, B2, etc.). */ + if (nparities >= nstripes) + return grub_error (GRUB_ERR_BAD_FS, + "invalid RAID5/6: nparities >= nstripes"); high = grub_divmod64 (stripe_nr, nstripes - nparities, &stripen); /* -- 2.11.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 046/117] commands/probe: Fix a resource leak when probing disks
From: Darren Kenny
Every other return statement in this code is calling grub_device_close()
to clean up dev before returning. This one should do that too.
Fixes: CID 292443
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
grub-core/commands/probe.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/grub-core/commands/probe.c b/grub-core/commands/probe.c
index 573bdf603..e53b61178 100644
--- a/grub-core/commands/probe.c
+++ b/grub-core/commands/probe.c
@@ -111,7 +111,11 @@ grub_cmd_probe (grub_extcmd_context_t ctxt, int argc, char
**args)
grub_disk_t disk = grub_disk_open(dev->disk->name);
if (!disk)
- return grub_errno;
+ {
+ grub_device_close (dev);
+ return grub_errno;
+ }
+
if (grub_strcmp(dev->disk->partition->partmap->name, "gpt") == 0)
{
struct grub_gpt_partentry entry;
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 050/117] video/fb/video_fb: Fix possible integer overflow
From: Darren Kenny
It is minimal possibility that the values being used here will overflow.
So, change the code to use the safemath function grub_mul() to ensure
that doesn't happen.
Fixes: CID 73761
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
grub-core/video/fb/video_fb.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/grub-core/video/fb/video_fb.c b/grub-core/video/fb/video_fb.c
index 1c9a138dc..ae6b89f9a 100644
--- a/grub-core/video/fb/video_fb.c
+++ b/grub-core/video/fb/video_fb.c
@@ -1537,7 +1537,13 @@ doublebuf_pageflipping_init (struct grub_video_mode_info
*mode_info,
volatile void *page1_ptr)
{
grub_err_t err;
- grub_size_t page_size = mode_info->pitch * mode_info->height;
+ grub_size_t page_size = 0;
+
+ if (grub_mul (mode_info->pitch, mode_info->height, &page_size))
+{
+ /* Shouldn't happen, but if it does we've a bug. */
+ return GRUB_ERR_BUG;
+}
framebuffer.offscreen_buffer = grub_malloc (page_size);
if (! framebuffer.offscreen_buffer)
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 074/117] fs/hfs: Disable under lockdown
From: Daniel Axtens
HFS has issues such as infinite mutual recursion that are simply too
complex to fix for such a legacy format. So simply do not permit
it to be loaded under lockdown.
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/fs/hfs.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/grub-core/fs/hfs.c b/grub-core/fs/hfs.c
index 3fe842b4d..9a5b7bbe9 100644
--- a/grub-core/fs/hfs.c
+++ b/grub-core/fs/hfs.c
@@ -30,6 +30,7 @@
#include
#include
#include
+#include
GRUB_MOD_LICENSE ("GPLv3+");
@@ -1433,11 +1434,13 @@ static struct grub_fs grub_hfs_fs =
GRUB_MOD_INIT(hfs)
{
- grub_fs_register (&grub_hfs_fs);
+ if (!grub_is_lockdown ())
+grub_fs_register (&grub_hfs_fs);
my_mod = mod;
}
GRUB_MOD_FINI(hfs)
{
- grub_fs_unregister (&grub_hfs_fs);
+ if (!grub_is_lockdown())
+grub_fs_unregister (&grub_hfs_fs);
}
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 092/117] disk/lvm: Do not allow a LV to be it's own segment's node's LV
From: Daniel Axtens
This prevents infinite recursion in the diskfilter verification code.
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/disk/lvm.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/grub-core/disk/lvm.c b/grub-core/disk/lvm.c
index 4e17b8897..8257159b3 100644
--- a/grub-core/disk/lvm.c
+++ b/grub-core/disk/lvm.c
@@ -981,9 +981,13 @@ grub_lvm_detect (grub_disk_t disk,
}
if (lv1->segments[i].nodes[j].pv == NULL)
for (lv2 = vg->lvs; lv2; lv2 = lv2->next)
- if (grub_strcmp (lv2->name,
-lv1->segments[i].nodes[j].name) == 0)
- lv1->segments[i].nodes[j].lv = lv2;
+ {
+ if (lv1 == lv2)
+ continue;
+ if (grub_strcmp (lv2->name,
+ lv1->segments[i].nodes[j].name) == 0)
+ lv1->segments[i].nodes[j].lv = lv2;
+ }
}
}
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 054/117] loader/xnu: Fix memory leak
From: Darren Kenny The code here is finished with the memory stored in name, but it only frees it if there curvalue is valid, while it could actually free it regardless. The fix is a simple relocation of the grub_free() to before the test of curvalue. Fixes: CID 96646 Signed-off-by: Darren Kenny Reviewed-by: Daniel Kiper --- grub-core/loader/xnu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/grub-core/loader/xnu.c b/grub-core/loader/xnu.c index 44fd5a979..eb1446251 100644 --- a/grub-core/loader/xnu.c +++ b/grub-core/loader/xnu.c @@ -1391,9 +1391,9 @@ grub_xnu_fill_devicetree (void) name[len] = 0; curvalue = grub_xnu_create_value (curkey, name); -if (!curvalue) - return grub_errno; grub_free (name); +if (!curvalue) + return grub_errno; data = grub_malloc (grub_strlen (var->value) + 1); if (!data) -- 2.11.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 059/117] util/glue-efi: Fix incorrect use of a possibly negative value
From: Darren Kenny
It is possible for the ftell() function to return a negative value,
although it is fairly unlikely here, we should be checking for
a negative value before we assign it to an unsigned value.
Fixes: CID 73744
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
util/glue-efi.c | 14 --
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/util/glue-efi.c b/util/glue-efi.c
index 68f53168b..de0fa6d33 100644
--- a/util/glue-efi.c
+++ b/util/glue-efi.c
@@ -39,13 +39,23 @@ write_fat (FILE *in32, FILE *in64, FILE *out, const char
*out_filename,
struct grub_macho_fat_header head;
struct grub_macho_fat_arch arch32, arch64;
grub_uint32_t size32, size64;
+ long size;
char *buf;
fseek (in32, 0, SEEK_END);
- size32 = ftell (in32);
+ size = ftell (in32);
+ if (size < 0)
+grub_util_error ("cannot get end of input file '%s': %s",
+name32, strerror (errno));
+ size32 = (grub_uint32_t) size;
fseek (in32, 0, SEEK_SET);
+
fseek (in64, 0, SEEK_END);
- size64 = ftell (in64);
+ size = ftell (in64);
+ if (size < 0)
+grub_util_error ("cannot get end of input file '%s': %s",
+name64, strerror (errno));
+ size64 = (grub_uint64_t) size;
fseek (in64, 0, SEEK_SET);
head.magic = grub_cpu_to_le32_compile_time (GRUB_MACHO_FAT_EFI_MAGIC);
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 103/117] util/mkimage: Use grub_host_to_target32() instead of grub_cpu_to_le32()
From: Peter Jones The latter doesn't take into account the target image endianness. There is a grub_cpu_to_le32_compile_time() but no compile time variant for function grub_host_to_target32(). So, let's keep using the other one for this case. Signed-off-by: Peter Jones Signed-off-by: Javier Martinez Canillas Reviewed-by: Daniel Kiper --- util/mkimage.c | 44 ++-- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/util/mkimage.c b/util/mkimage.c index 32bb8ea68..02944f28e 100644 --- a/util/mkimage.c +++ b/util/mkimage.c @@ -1302,10 +1302,10 @@ grub_install_generate_image (const char *dir, const char *prefix, + sizeof (struct grub_pe32_coff_header)); o->magic = grub_host_to_target16 (GRUB_PE32_PE32_MAGIC); o->code_size = grub_host_to_target32 (layout.exec_size); - o->data_size = grub_cpu_to_le32 (reloc_addr - layout.exec_size + o->data_size = grub_host_to_target32 (reloc_addr - layout.exec_size - header_size); - o->entry_addr = grub_cpu_to_le32 (layout.start_address); - o->code_base = grub_cpu_to_le32 (header_size); + o->entry_addr = grub_host_to_target32 (layout.start_address); + o->code_base = grub_host_to_target32 (header_size); o->data_base = grub_host_to_target32 (header_size + layout.exec_size); @@ -1339,10 +1339,10 @@ grub_install_generate_image (const char *dir, const char *prefix, + sizeof (struct grub_pe32_coff_header)); o->magic = grub_host_to_target16 (GRUB_PE32_PE64_MAGIC); o->code_size = grub_host_to_target32 (layout.exec_size); - o->data_size = grub_cpu_to_le32 (reloc_addr - layout.exec_size + o->data_size = grub_host_to_target32 (reloc_addr - layout.exec_size - header_size); - o->entry_addr = grub_cpu_to_le32 (layout.start_address); - o->code_base = grub_cpu_to_le32 (header_size); + o->entry_addr = grub_host_to_target32 (layout.start_address); + o->code_base = grub_host_to_target32 (header_size); o->image_base = 0; o->section_alignment = grub_host_to_target32 (image_target->section_align); o->file_alignment = grub_host_to_target32 (GRUB_PE32_FILE_ALIGNMENT); @@ -1366,10 +1366,10 @@ grub_install_generate_image (const char *dir, const char *prefix, /* The sections. */ text_section = sections; strcpy (text_section->name, ".text"); - text_section->virtual_size = grub_cpu_to_le32 (layout.exec_size); - text_section->virtual_address = grub_cpu_to_le32 (header_size); - text_section->raw_data_size = grub_cpu_to_le32 (layout.exec_size); - text_section->raw_data_offset = grub_cpu_to_le32 (header_size); + text_section->virtual_size = grub_host_to_target32 (layout.exec_size); + text_section->virtual_address = grub_host_to_target32 (header_size); + text_section->raw_data_size = grub_host_to_target32 (layout.exec_size); + text_section->raw_data_offset = grub_host_to_target32 (header_size); text_section->characteristics = grub_cpu_to_le32_compile_time ( GRUB_PE32_SCN_CNT_CODE | GRUB_PE32_SCN_MEM_EXECUTE @@ -1377,10 +1377,10 @@ grub_install_generate_image (const char *dir, const char *prefix, data_section = text_section + 1; strcpy (data_section->name, ".data"); - data_section->virtual_size = grub_cpu_to_le32 (layout.kernel_size - layout.exec_size); - data_section->virtual_address = grub_cpu_to_le32 (header_size + layout.exec_size); - data_section->raw_data_size = grub_cpu_to_le32 (layout.kernel_size - layout.exec_size); - data_section->raw_data_offset = grub_cpu_to_le32 (header_size + layout.exec_size); + data_section->virtual_size = grub_host_to_target32 (layout.kernel_size - layout.exec_size); + data_section->virtual_address = grub_host_to_target32 (header_size + layout.exec_size); + data_section->raw_data_size = grub_host_to_target32 (layout.kernel_size - layout.exec_size); + data_section->raw_data_offset = grub_host_to_target32 (header_size + layout.exec_size); data_section->characteristics = grub_cpu_to_le32_compile_time (GRUB_PE32_SCN_CNT_INITIALIZED_DATA | GRUB_PE32_SCN_MEM_READ @@ -1388,10 +1388,10 @@ grub_install_generate_image (const char *dir, const char *prefix, mods_section = data_section + 1; strcpy (mods_section->name, "mods"); - mods_section->virtual_size = grub_cpu_to_le32 (reloc_addr - layout.kernel_size - header_size); - mods_section->virtual_address = grub_cpu_to_le32 (header_size + layout.kernel_size + layout.bss_size); - mods_section->raw_data_size =
