Michael Chang [Thu, 18 Mar 2021 11:30:26 +0000 (19:30 +0800)]
i386-pc: build verifiers API as module
Given no core functions on i386-pc would require verifiers to work and
the only consumer of the verifier API is the pgp module, it looks good
to me that we can move the verifiers out of the kernel image and let
moddep.lst to auto-load it when pgp is loaded on i386-pc platform.
This helps to reduce the size of core image and thus can relax the
tension of exploding on some i386-pc system with very short MBR gap
size. See also a very comprehensive summary from Colin [1] about the
details.
V2:
Drop COND_NOT_i386_pc and use !COND_i386_pc.
Add comment in kern/verifiers.c to help understanding what's going on
without digging into the commit history.
Reported-by: Colin Watson <cjwatson@debian.org> Reviewed-by: Colin Watson <cjwatson@debian.org> Signed-off-by: Michael Chang <mchang@suse.com>
Origin: other, https://lists.gnu.org/archive/html/grub-devel/2021-03/msg00251.html
Bug-Debian: https://bugs.debian.org/984488
Bug-Debian: https://bugs.debian.org/985374
Last-Update: 2021-03-18
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 <mbenatto@redhat.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/113-kern-mm-Fix-grub_debug_calloc-compilation-error.patch
gfxmenu/gui: Check printf() format in the gui_progress_bar and gui_label
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 <tf@miray.de> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/112-gfxmenu-gui-Check-printf-format-in-the-gui_progress_bar-and-gui_label.patch
kern/misc: Add function to check printf() format against expected format
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 <tf@miray.de> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/111-kern-misc-Add-function-to-check-printf-format-against-expected-format.patch
kern/misc: Add STRING type for internal printf() format handling
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 <tf@miray.de> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/110-kern-misc-Add-STRING-type-for-internal-printf-format-handling.patch
kern/misc: Split parse_printf_args() into format parsing and va_list handling
This patch is preparing for a follow up patch which will use
the format parsing part to compare the arguments in a printf()
format from an external source against a printf() format with
expected arguments.
Signed-off-by: Thomas Frauendorfer | Miray Software <tf@miray.de> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/109-kern-misc-Split-parse_printf_args-into-format-parsing-and-va_list-handling.patch
Peter Jones [Mon, 15 Feb 2021 16:07:00 +0000 (17:07 +0100)]
util/mkimage: Add an option to import SBAT metadata into a .sbat section
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 <pjones@redhat.com> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/107-util-mkimage-Add-an-option-to-import-SBAT-metadata-into-a-.sbat-section.patch
Peter Jones [Mon, 15 Feb 2021 13:58:06 +0000 (14:58 +0100)]
util/mkimage: Refactor section setup to use a helper
Add a init_pe_section() helper function to setup PE sections. This makes
the code simpler and easier to read.
Signed-off-by: Peter Jones <pjones@redhat.com> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/106-util-mkimage-Refactor-section-setup-to-use-a-helper.patch
Peter Jones [Thu, 11 Feb 2021 16:07:33 +0000 (17:07 +0100)]
util/mkimage: Improve data_size value calculation
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 <pjones@redhat.com> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/105-util-mkimage-Improve-data_size-value-calculation.patch
Peter Jones [Mon, 15 Feb 2021 13:21:48 +0000 (14:21 +0100)]
util/mkimage: Reorder PE optional header fields set-up
This makes the PE32 and PE32+ header fields set-up easier to follow by
setting them closer to the initialization of their related sections.
Signed-off-by: Peter Jones <pjones@redhat.com> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/104-util-mkimage-Reorder-PE-optional-header-fields-set-up.patch
Peter Jones [Mon, 15 Feb 2021 13:19:31 +0000 (14:19 +0100)]
util/mkimage: Unify more of the PE32 and PE32+ header set-up
There's quite a bit of code duplication in the code that sets the optional
header for PE32 and PE32+. The two are very similar with the exception of
a few fields that have type grub_uint64_t instead of grub_uint32_t.
Factor out the common code and add a PE_OHDR() macro that simplifies the
set-up and make the code more readable.
Signed-off-by: Peter Jones <pjones@redhat.com> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/103-util-mkimage-Unify-more-of-the-PE32-and-PE32-header-set-up.patch
Peter Jones [Mon, 15 Feb 2021 13:14:24 +0000 (14:14 +0100)]
util/mkimage: Always use grub_host_to_target32() to initialize PE stack and heap stuff
This change does not impact final result of initialization itself.
However, it eases PE code unification in subsequent patches.
Signed-off-by: Peter Jones <pjones@redhat.com> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/102-util-mkimage-Always-use-grub_host_to_target32-to-initialize-PE-stack-and-heap-stuff.patch
Peter Jones [Mon, 15 Feb 2021 12:59:21 +0000 (13:59 +0100)]
util/mkimage: Use grub_host_to_target32() instead of grub_cpu_to_le32()
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 <pjones@redhat.com> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/101-util-mkimage-Use-grub_host_to_target32-instead-of-grub_cpu_to_le32.patch
It works only on UEFI platforms but can be quite easily extended to
others architectures and platforms if needed.
Signed-off-by: Chris Coulson <chris.coulson@canonical.com> Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com> Reviewed-by: Marco A Benatto <mbenatto@redhat.com> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Patch-Name: 2021-02-security/099-kern-efi-Add-initial-stack-protector-implementation.patch
Chris Coulson [Thu, 7 Jan 2021 19:21:03 +0000 (19:21 +0000)]
kern/parser: Fix a stack buffer overflow
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 <chris.coulson@canonical.com> Signed-off-by: Chris Coulson <chris.coulson@canonical.com> Signed-off-by: Darren Kenny <darren.kenny@oracle.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/098-kern-parser-Fix-a-stack-buffer-overflow.patch
Chris Coulson [Thu, 7 Jan 2021 15:15:43 +0000 (15:15 +0000)]
kern/buffer: Add variable sized heap buffer
Add a new variable sized heap buffer type (grub_buffer_t) with simple
operations for appending data, accessing the data and maintaining
a read cursor.
Signed-off-by: Chris Coulson <chris.coulson@canonical.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/097-kern-buffer-Add-variable-sized-heap-buffer.patch
Introduce a common function epilogue used for cleaning up on all
return paths, which will simplify additional error handling to be
introduced in a subsequent commit.
Signed-off-by: Chris Coulson <chris.coulson@canonical.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/096-kern-parser-Refactor-grub_parser_split_cmdline-cleanup.patch
Chris Coulson [Thu, 7 Jan 2021 19:53:55 +0000 (19:53 +0000)]
kern/parser: Introduce terminate_arg() helper
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 <chris.coulson@canonical.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/095-kern-parser-Introduce-terminate_arg-helper.patch
Chris Coulson [Tue, 5 Jan 2021 22:17:28 +0000 (22:17 +0000)]
kern/parser: Introduce process_char() helper
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 <chris.coulson@canonical.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/094-kern-parser-Introduce-process_char-helper.patch
Chris Coulson [Wed, 18 Nov 2020 00:59:24 +0000 (00:59 +0000)]
kern/parser: Fix a memory leak
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 <chris.coulson@canonical.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/093-kern-parser-Fix-a-memory-leak.patch
Daniel Axtens [Mon, 18 Jan 2021 06:27:18 +0000 (17:27 +1100)]
fs/btrfs: Squash some uninitialized reads
We need to check errors before calling into a function that uses the result.
Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/092-fs-btrfs-Squash-some-uninitialized-reads.patch
Daniel Axtens [Mon, 18 Jan 2021 06:17:16 +0000 (17:17 +1100)]
fs/btrfs: Validate the number of stripes/parities in RAID5/6
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 <dja@axtens.net> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/091-fs-btrfs-Validate-the-number-of-stripes-parities-in-RAID5-6.patch
Daniel Axtens [Fri, 22 Jan 2021 03:42:21 +0000 (14:42 +1100)]
disk/lvm: Do not allow a LV to be it's own segment's node's LV
This prevents infinite recursion in the diskfilter verification code.
Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/090-disk-lvm-Do-not-allow-a-LV-to-be-it-s-own-segment-s-node-s-LV.patch
Daniel Axtens [Fri, 22 Jan 2021 03:43:58 +0000 (14:43 +1100)]
disk/lvm: Sanitize rlocn->offset to prevent wild read
rlocn->offset is read directly from disk and added to the metadatabuf
pointer to create a pointer to a block of metadata. It's a 64-bit
quantity so as long as you don't overflow you can set subsequent
pointers to point anywhere in memory.
Require that rlocn->offset fits within the metadata buffer size.
Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/089-disk-lvm-Sanitize-rlocn-offset-to-prevent-wild-read.patch
Daniel Axtens [Thu, 21 Jan 2021 07:35:22 +0000 (18:35 +1100)]
disk/lvm: Do not overread metadata
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 <dja@axtens.net> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/088-disk-lvm-Do-not-overread-metadata.patch
Daniel Axtens [Thu, 21 Jan 2021 07:35:22 +0000 (18:35 +1100)]
disk/lvm: Do not crash if an expected string is not found
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 <dja@axtens.net> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/087-disk-lvm-Do-not-crash-if-an-expected-string-is-not-found.patch
Daniel Axtens [Thu, 21 Jan 2021 07:54:29 +0000 (18:54 +1100)]
disk/lvm: Bail on missing PV list
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 <dja@axtens.net> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/086-disk-lvm-Bail-on-missing-PV-list.patch
Daniel Axtens [Thu, 21 Jan 2021 07:19:51 +0000 (18:19 +1100)]
disk/lvm: Don't blast past the end of the circular metadata buffer
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 <dja@axtens.net> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/085-disk-lvm-Don-t-blast-past-the-end-of-the-circular-metadata-buffer.patch
Daniel Axtens [Thu, 21 Jan 2021 06:59:14 +0000 (17:59 +1100)]
disk/lvm: Don't go beyond the end of the data we read from disk
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 <dja@axtens.net> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/084-disk-lvm-Don-t-go-beyond-the-end-of-the-data-we-read-from-disk.patch
Daniel Axtens [Thu, 21 Jan 2021 01:22:28 +0000 (12:22 +1100)]
io/gzio: Zero gzio->tl/td in init_dynamic_block() if huft_build() fails
If huft_build() fails, gzio->tl or gzio->td could contain pointers that
are no longer valid. Zero them out.
This prevents a double free when grub_gzio_close() comes through and
attempts to free them again.
Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/083-io-gzio-Zero-gzio-tl-td-in-init_dynamic_block-if-huft_build-fails.patch
Daniel Axtens [Thu, 21 Jan 2021 01:20:49 +0000 (12:20 +1100)]
io/gzio: Catch missing values in huft_build() and bail
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 <dja@axtens.net> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/082-io-gzio-Catch-missing-values-in-huft_build-and-bail.patch
Daniel Axtens [Wed, 20 Jan 2021 13:05:58 +0000 (00:05 +1100)]
io/gzio: Add init_dynamic_block() clean up if unpacking codes fails
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 <dja@axtens.net> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/081-io-gzio-Add-init_dynamic_block-clean-up-if-unpacking-codes-fails.patch
Daniel Axtens [Wed, 13 Jan 2021 09:59:09 +0000 (20:59 +1100)]
io/gzio: Bail if gzio->tl/td is NULL
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 <dja@axtens.net> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/080-io-gzio-Bail-if-gzio-tl-td-is-NULL.patch
Daniel Axtens [Mon, 18 Jan 2021 06:06:19 +0000 (17:06 +1100)]
fs/nilfs2: Properly bail on errors in grub_nilfs2_btree_node_lookup()
We just introduced an error return in grub_nilfs2_btree_node_lookup().
Make sure the callers catch it.
At the same time, make sure that grub_nilfs2_btree_node_lookup() always
inits the index pointer passed to it.
Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/079-fs-nilfs2-Properly-bail-on-errors-in-grub_nilfs2_btree_node_lookup.patch
Daniel Axtens [Mon, 18 Jan 2021 05:49:44 +0000 (16:49 +1100)]
fs/nilfs2: Don't search children if provided number is too large
NILFS2 reads the number of children a node has from the node. Unfortunately,
that's not trustworthy. Check if it's beyond what the filesystem permits and
reject it if so.
This blocks some OOB reads. I'm not sure how controllable the read is and what
could be done with invalidly read data later on.
Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/078-fs-nilfs2-Don-t-search-children-if-provided-number-is-too-large.patch
Daniel Axtens [Mon, 18 Jan 2021 05:49:09 +0000 (16:49 +1100)]
fs/nilfs2: Reject too-large keys
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 <dja@axtens.net> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/077-fs-nilfs2-Reject-too-large-keys.patch
Daniel Axtens [Mon, 18 Jan 2021 04:47:24 +0000 (15:47 +1100)]
fs/jfs: Catch infinite recursion
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 <dja@axtens.net> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/076-fs-jfs-Catch-infinite-recursion.patch
Daniel Axtens [Mon, 18 Jan 2021 03:57:17 +0000 (14:57 +1100)]
fs/jfs: Limit the extents that getblk() can consider
getblk() implicitly trusts that treehead->count is an accurate count of
the number of extents. However, that value is read from disk and is not
trustworthy, leading to OOB reads and crashes. I am not sure to what
extent the data read from OOB can influence subsequent program execution.
Require callers to pass in the maximum number of extents for which
they have storage.
Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/075-fs-jfs-Limit-the-extents-that-getblk-can-consider.patch
Daniel Axtens [Mon, 18 Jan 2021 03:51:11 +0000 (14:51 +1100)]
fs/jfs: Do not move to leaf level if name length is negative
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 <dja@axtens.net> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/074-fs-jfs-Do-not-move-to-leaf-level-if-name-length-is-negative.patch
Daniel Axtens [Mon, 18 Jan 2021 03:34:58 +0000 (14:34 +1100)]
fs/sfs: Fix over-read of root object name
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 <dja@axtens.net> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/073-fs-sfs-Fix-over-read-of-root-object-name.patch
Daniel Axtens [Mon, 18 Jan 2021 01:19:07 +0000 (12:19 +1100)]
fs/hfs: Disable under lockdown
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 <dja@axtens.net> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/072-fs-hfs-Disable-under-lockdown.patch
Daniel Axtens [Tue, 2 Feb 2021 05:59:35 +0000 (16:59 +1100)]
fs/hfsplus: Don't use uninitialized data on corrupt filesystems
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 <dja@axtens.net> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/071-fs-hfsplus-Don-t-use-uninitialized-data-on-corrupt-filesystems.patch
Daniel Axtens [Fri, 22 Jan 2021 07:13:56 +0000 (18:13 +1100)]
fs/hfsplus: Don't fetch a key beyond the end of the node
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 <dja@axtens.net> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/070-fs-hfsplus-Don-t-fetch-a-key-beyond-the-end-of-the-node.patch
Daniel Axtens [Mon, 18 Jan 2021 00:46:39 +0000 (11:46 +1100)]
fs/fshelp: Catch impermissibly large block sizes in read helper
A fuzzed HFS+ filesystem had log2blocksize = 22. This gave
log2blocksize + GRUB_DISK_SECTOR_BITS = 31. 1 << 31 = 0x80000000,
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 <dja@axtens.net> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/069-fs-fshelp-Catch-impermissibly-large-block-sizes-in-read-helper.patch
Daniel Axtens [Fri, 15 Jan 2021 09:03:20 +0000 (20:03 +1100)]
term/gfxterm: Don't set up a font with glyphs that are too big
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 <dja@axtens.net> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/068-term-gfxterm-Don-t-set-up-a-font-with-glyphs-that-are-too-big.patch
Daniel Axtens [Fri, 15 Jan 2021 03:06:46 +0000 (14:06 +1100)]
video/readers/jpeg: Don't decode data before start of stream
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 <dja@axtens.net> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/067-video-readers-jpeg-Don-t-decode-data-before-start-of-stream.patch
Daniel Axtens [Fri, 15 Jan 2021 02:29:53 +0000 (13:29 +1100)]
video/readers/jpeg: Catch OOB reads/writes in grub_jpeg_decode_du()
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 <dja@axtens.net> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/066-video-readers-jpeg-Catch-OOB-reads-writes-in-grub_jpeg_decode_du.patch
Daniel Axtens [Fri, 15 Jan 2021 01:57:04 +0000 (12:57 +1100)]
video/readers/jpeg: Catch files with unsupported quantization or Huffman tables
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 <dja@axtens.net> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/065-video-readers-jpeg-Catch-files-with-unsupported-quantization-or-Huffman-tables.patch
Daniel Axtens [Wed, 13 Jan 2021 11:19:01 +0000 (22:19 +1100)]
kern/misc: Always set *end in grub_strtoull()
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 <dja@axtens.net> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/064-kern-misc-Always-set-end-in-grub_strtoull.patch
Daniel Axtens [Fri, 22 Jan 2021 06:10:48 +0000 (17:10 +1100)]
commands/menuentry: Fix quoting in setparams_prefix()
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: CVE-2021-20233 Fixes: 9acdcbf32542 (use single quotes in menuentry setparams command) Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/063-commands-menuentry-Fix-quoting-in-setparams_prefix.patch
Daniel Axtens [Fri, 22 Jan 2021 05:18:26 +0000 (16:18 +1100)]
script/execute: Don't crash on a "for" loop with no items
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 <dja@axtens.net> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/062-script-execute-Don-t-crash-on-a-for-loop-with-no-items.patch
Daniel Axtens [Fri, 22 Jan 2021 05:07:29 +0000 (16:07 +1100)]
lib/arg: Block repeated short options that require an argument
Fuzzing found the following crash:
search -hhhhhhhhhhhhhf
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 Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/061-lib-arg-Block-repeated-short-options-that-require-an-argument.patch
Daniel Axtens [Mon, 11 Jan 2021 06:30:42 +0000 (17:30 +1100)]
script/execute: Avoid crash when using "$#" outside a function scope
"$#" 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 <dja@axtens.net> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/060-script-execute-Avoid-crash-when-using-outside-a-function-scope.patch
Daniel Axtens [Mon, 11 Jan 2021 05:57:37 +0000 (16:57 +1100)]
commands/ls: Require device_name is not NULL before printing
This can be triggered with:
ls -l (0 0*)
and causes a NULL deref in grub_normal_print_device_info().
I'm not sure if there's any implication with the IEEE 1275 platform.
Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/059-commands-ls-Require-device_name-is-not-NULL-before-printing.patch
Daniel Axtens [Fri, 3 Apr 2020 12:05:13 +0000 (23:05 +1100)]
script/execute: Fix NULL dereference in grub_script_execute_cmdline()
Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/058-script-execute-Fix-NULL-dereference-in-grub_script_execute_cmdline.patch
Darren Kenny [Fri, 4 Dec 2020 15:04:28 +0000 (15:04 +0000)]
util/glue-efi: Fix incorrect use of a possibly negative value
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 <darren.kenny@oracle.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/057-util-glue-efi-Fix-incorrect-use-of-a-possibly-negative-value.patch
Darren Kenny [Thu, 5 Nov 2020 14:33:50 +0000 (14:33 +0000)]
util/grub-editenv: Fix incorrect casting of a signed value
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 <darren.kenny@oracle.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/056-util-grub-editenv-Fix-incorrect-casting-of-a-signed-value.patch
Marco A Benatto [Mon, 30 Nov 2020 15:18:24 +0000 (12:18 -0300)]
loader/xnu: Free driverkey data when an error is detected in grub_xnu_writetree_toheap()
... to avoid memory leaks.
Fixes: CID 96640 Signed-off-by: Marco A Benatto <mbenatto@redhat.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/053-loader-xnu-Free-driverkey-data-when-an-error-is-detected-in-grub_xnu_writetree_toheap.patch
Darren Kenny [Mon, 7 Dec 2020 14:44:47 +0000 (14:44 +0000)]
gfxmenu/gui_list: Remove code that coverity is flagging as dead
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 <darren.kenny@oracle.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/050-gfxmenu-gui_list-Remove-code-that-coverity-is-flagging-as-dead.patch
Darren Kenny [Fri, 4 Dec 2020 15:39:00 +0000 (15:39 +0000)]
video/readers/jpeg: Test for an invalid next marker reference from a jpeg file
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 <darren.kenny@oracle.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/049-video-readers-jpeg-Test-for-an-invalid-next-marker-reference-from-a-jpeg-file.patch
Darren Kenny [Fri, 4 Dec 2020 14:51:30 +0000 (14:51 +0000)]
video/fb/video_fb: Fix possible integer overflow
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 <darren.kenny@oracle.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/048-video-fb-video_fb-Fix-possible-integer-overflow.patch
Darren Kenny [Wed, 4 Nov 2020 14:43:44 +0000 (14:43 +0000)]
video/fb/video_fb: Fix multiple integer overflows
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 <darren.kenny@oracle.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/047-video-fb-video_fb-Fix-multiple-integer-overflows.patch
Darren Kenny [Wed, 4 Nov 2020 15:10:51 +0000 (15:10 +0000)]
video/fb/fbfill: Fix potential integer overflow
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 <darren.kenny@oracle.com> Signed-off-by: Marco A Benatto <mbenatto@redhat.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/046-video-fb-fbfill-Fix-potential-integer-overflow.patch
Darren Kenny [Tue, 8 Dec 2020 21:14:31 +0000 (21:14 +0000)]
video/efi_gop: Remove unnecessary return value of grub_video_gop_fill_mode_info()
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 <darren.kenny@oracle.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/045-video-efi_gop-Remove-unnecessary-return-value-of-grub_video_gop_fill_mode_info.patch
Chris Coulson [Tue, 1 Dec 2020 23:41:24 +0000 (23:41 +0000)]
commands/hashsum: Fix a memory leak
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 <chris.coulson@canonical.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/044-commands-hashsum-Fix-a-memory-leak.patch
Darren Kenny [Fri, 4 Dec 2020 18:56:48 +0000 (18:56 +0000)]
normal/completion: Fix leaking of memory when processing a completion
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 <darren.kenny@oracle.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/043-normal-completion-Fix-leaking-of-memory-when-processing-a-completion.patch
Darren Kenny [Tue, 3 Nov 2020 16:43:37 +0000 (16:43 +0000)]
libgcrypt/mpi: Fix possible unintended sign extension
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
Darren Kenny [Thu, 26 Nov 2020 12:48:07 +0000 (12:48 +0000)]
affs: Fix memory leaks
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
Darren Kenny [Thu, 26 Nov 2020 10:56:45 +0000 (10:56 +0000)]
zfsinfo: Correct a check for error allocating memory
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
Darren Kenny [Tue, 8 Dec 2020 22:17:04 +0000 (22:17 +0000)]
zfs: Fix possible integer overflows
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
Darren Kenny [Tue, 24 Nov 2020 16:41:49 +0000 (16:41 +0000)]
zfs: Fix possible negative shift operation
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
Darren Kenny [Thu, 21 Jan 2021 11:38:31 +0000 (11:38 +0000)]
disk/cryptodisk: Fix potential integer overflow
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
Marco A Benatto [Mon, 7 Dec 2020 14:53:03 +0000 (11:53 -0300)]
disk/ldm: Make sure comp data is freed before exiting from make_vg()
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
Darren Kenny [Fri, 23 Oct 2020 09:49:59 +0000 (09:49 +0000)]
kern/partition: Check for NULL before dereferencing input string
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
Darren Kenny [Thu, 5 Nov 2020 10:29:59 +0000 (10:29 +0000)]
zstd: Initialize seq_t structure fully
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
Darren Kenny [Tue, 24 Nov 2020 18:04:22 +0000 (18:04 +0000)]
gnulib/regcomp: Fix uninitialized re_token
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
Darren Kenny [Thu, 5 Nov 2020 10:57:14 +0000 (10:57 +0000)]
gnulib/regexec: Fix possible null-dereference
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
Darren Kenny [Wed, 28 Oct 2020 14:43:01 +0000 (14:43 +0000)]
gnulib/argp-help: Fix dereference of a possibly NULL state
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
Darren Kenny [Wed, 21 Oct 2020 14:41:27 +0000 (14:41 +0000)]
gnulib/regexec: Resolve unused variable
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
Darren Kenny [Fri, 11 Dec 2020 15:03:13 +0000 (15:03 +0000)]
kern/efi/mm: Fix possible NULL pointer dereference
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
Darren Kenny [Fri, 22 Jan 2021 12:32:41 +0000 (12:32 +0000)]
kern/parser: Fix resource leak if argc == 0
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
Darren Kenny [Fri, 19 Feb 2021 17:12:23 +0000 (17:12 +0000)]
net/tftp: Fix dangling memory pointer
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
projects / grub2.git / log