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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
Commit 9acdcbf325 (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 22e7dbb2bb (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: 9acdcbf325 (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
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
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
"$#" 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
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
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
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
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
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 <daniel.kiper@oracle.com>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Patch-Name: 2021-02-security/055-util-grub-install-Fix-NULL-pointer-dereferences.patch
... 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
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 <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/052-loader-xnu-Fix-memory-leak.patch
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 <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Patch-Name: 2021-02-security/051-loader-bsd-Check-for-NULL-arg-up-front.patch
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
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
