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 <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/040-libgcrypt-mpi-Fix-possible-unintended-sign-extension.patch
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 <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/039-affs-Fix-memory-leaks.patch
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 <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/038-zfsinfo-Correct-a-check-for-error-allocating-memory.patch
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 <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/037-zfs-Fix-possible-integer-overflows.patch
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 <pfsmorigo@canonical.com>
Signed-off-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/036-zfs-Fix-resource-leaks-while-constructing-path.patch
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 <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/035-zfs-Fix-possible-negative-shift-operation.patch
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 <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/034-hfsplus-Check-that-the-volume-name-length-is-valid.patch
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 <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/033-disk-cryptodisk-Fix-potential-integer-overflow.patch
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 <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/032-disk-ldm-Fix-memory-leak-on-uninserted-lv-references.patch
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 <mbenatto@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/030-disk-ldm-Make-sure-comp-data-is-freed-before-exiting-from-make_vg.patch
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 <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/029-kern-partition-Check-for-NULL-before-dereferencing-input-string.patch
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 <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/028-zstd-Initialize-seq_t-structure-fully.patch
These 2 assignments are unnecessary since they are just assigning
to themselves.
Fixes: CID 73643
Signed-off-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/027-io-lzopio-Resolve-unnecessary-self-assignment-errors.patch
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 <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/026-gnulib-regcomp-Fix-uninitialized-re_token.patch
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 <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/025-gnulib-regexec-Fix-possible-null-dereference.patch
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 <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/024-gnulib-argp-help-Fix-dereference-of-a-possibly-NULL-state.patch
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 <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/023-gnulib-regcomp-Fix-uninitialized-token-structure.patch
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 <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/022-gnulib-regexec-Resolve-unused-variable.patch
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 <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/021-kern-efi-mm-Fix-possible-NULL-pointer-dereference.patch
Free the memory allocated to name before returning on failure.
Fixes: CID 296222
Signed-off-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/020-kern-efi-Fix-memory-leak-on-failure.patch
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 <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/019-kern-parser-Fix-resource-leak-if-argc-0.patch
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 <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/018-net-tftp-Fix-dangling-memory-pointer.patch
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 <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/017-net-net-Fix-possible-dereference-to-of-a-NULL-pointer.patch
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 <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/016-mmap-Fix-memory-leak-when-iterating-over-mapped-memory.patch
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 (IOActive)
Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/015-usb-Avoid-possible-out-of-bound-accesses-caused-by-malicious-devices.patch
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 <chris.coulson@canonical.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/014-dl-Only-allow-unloading-modules-that-are-not-dependencies.patch
The command is not present in the docs/grub.texi user documentation.
Reported-by: Daniel Kiper <daniel.kiper@oracle.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Patch-Name: 2021-02-security/013-docs-Document-the-cutmem-command.patch
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 <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/012-loader-xnu-Don-t-allow-loading-extension-and-packages-when-locked-down.patch
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 <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/011-gdb-Restrict-GDB-access-when-locked-down.patch
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 <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/010-commands-hdparm-Restrict-hdparm-command-when-locked-down.patch
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 <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/009-commands-setpci-Restrict-setpci-command-when-locked-down.patch
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 <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/008-commands-Restrict-commands-that-can-load-BIOS-or-DT-blobs-when-locked-down.patch
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 <teddy.reed@gmail.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/007-mmap-Don-t-register-cutmem-and-badram-commands-when-lockdown-is-enforced.patch
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 <km@mkukri.xyz>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/006-acpi-Don-t-register-the-acpi-command-when-locked-down.patch
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 <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/005-efi-Use-grub_is_lockdown-instead-of-hardcoding-a-disabled-modules-list.patch
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 <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/004-efi-Lockdown-the-GRUB-when-the-UEFI-Secure-Boot-is-enabled.patch
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 <xnox@ubuntu.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/003-kern-lockdown-Set-a-variable-if-the-GRUB-is-locked-down.patch
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 <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/002-kern-Add-lockdown-support.patch
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 <mbenatto@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/001-verifiers-Move-verifiers-API-to-kernel-image.patch
dpkg-reconfigure doesn't work when the package isn't fully installed.
Thanks to Daniel Leidert for pointing out the problem in comments on
https://bugs.debian.org/966575.
Commit 3d8439da8 (grub-install: Locale depends on nls) attempted to avoid
copying locale files to the target directory when NLS was disabled.
However the test is inverted, and it does the opposite.
Signed-off-by: Martin Whitaker <fsf@martin-whitaker.me.uk>
Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Origin: upstream, https://git.savannah.gnu.org/cgit/grub.git/commit/?id=8451454889f3b46312cc16eb07970d6c20da0812
Last-Update: 2021-02-07
Patch-Name: grub-install-inverted-nls-test.patch
Tools like simple-cdd ensures the presence of grub-efi on the generated
ISO image but don't have a simple way to to deal with architecture
specific packages.
Thus I'd like to suggest to keep grub-efi not as a transitional package
but as normal dummy package depending on the package most likely to be
appropriate for each architecture.
Closes: #981819
