Re: [PATCH 0/3] Add support for EFI file system transposition

2022-06-07 Thread Thomas Schmitt
Hi, Pete Batard wrote: > 2. It uses a efi.img to embed the UEFI bootloaders, but does not keep a copy > of these bootloaders on the ISO9660 file system itself, with the end result > that, when copying the media at the file system level, the '/efi/boot/' > directory and its content is missing. I u

Re: [PATCH 0/3] Add support for EFI file system transposition

2022-06-07 Thread Pete Batard
Hi Thomas. Thanks for replying. On 2022.06.07 08:16, Thomas Schmitt wrote: we will point out that we consider it should really be the job of xorriso, rather than grub-mkrescue, to accomplish this duplication (hence why I am CC'ing Thomas), but we don't know the technical difficulties that result

[SECURITY PATCH 00/30] Multiple GRUB2 vulnerabilities - 2022/06/07 round

2022-06-07 Thread Daniel Kiper
Hi all, This patch set contains a bundle of fixes for various security flaws discovered in the GRUB2 during last year. The most severe ones, i.e. potentially exploitable, have CVEs assigned and are listed at the end of this email. Additionally, the list of CVEs contains a CVE assigned for the sh

[SECURITY PATCH 02/30] commands/boot: Add API to pass context to loader

2022-06-07 Thread Daniel Kiper
From: Chris Coulson Loaders rely on global variables for saving context which is consumed in the boot hook and freed in the unload hook. In the case where a loader command is executed twice, calling grub_loader_set() a second time executes the unload hook, but in some cases this runs when the loa

[SECURITY PATCH 04/30] kern/efi/sb: Reject non-kernel files in the shim_lock verifier

2022-06-07 Thread Daniel Kiper
From: Julian Andres Klode We must not allow other verifiers to pass things like the GRUB modules. Instead of maintaining a blocklist, maintain an allowlist of things that we do not care about. This allowlist really should be made reusable, and shared by the lockdown verifier, but this is the min

[SECURITY PATCH 13/30] video/readers/jpeg: Refuse to handle multiple start of streams

2022-06-07 Thread Daniel Kiper
From: Daniel Axtens An invalid file could contain multiple start of stream blocks, which would cause us to reallocate and leak our bitmap. Refuse to handle multiple start of streams. Additionally, fix a grub_error() call formatting. Signed-off-by: Daniel Axtens Reviewed-by: Daniel Kiper ---

[SECURITY PATCH 15/30] normal/charset: Fix array out-of-bounds formatting unicode for display

2022-06-07 Thread Daniel Kiper
From: Daniel Axtens In some cases attempting to display arbitrary binary strings leads to ASAN splats reading the widthspec array out of bounds. Check the index. If it would be out of bounds, return a width of 1. I don't know if that's strictly correct, but we're not really expecting great displ

[SECURITY PATCH 01/30] loader/efi/chainloader: Simplify the loader state

2022-06-07 Thread Daniel Kiper
From: Chris Coulson The chainloader command retains the source buffer and device path passed to LoadImage(), requiring the unload hook passed to grub_loader_set() to free them. It isn't required to retain this state though - they aren't required by StartImage() or anything else in the boot hook,

[SECURITY PATCH 12/30] video/readers/jpeg: Do not reallocate a given huff table

2022-06-07 Thread Daniel Kiper
From: Daniel Axtens Fix a memory leak where an invalid file could cause us to reallocate memory for a huffman table we had already allocated memory for. Signed-off-by: Daniel Axtens Reviewed-by: Daniel Kiper --- grub-core/video/readers/jpeg.c | 3 +++ 1 file changed, 3 insertions(+) diff --g

[SECURITY PATCH 05/30] kern/file: Do not leak device_name on error in grub_file_open()

2022-06-07 Thread Daniel Kiper
From: Daniel Axtens If we have an error in grub_file_open() before we free device_name, we will leak it. Free device_name in the error path and null out the pointer in the good path once we free it there. Signed-off-by: Daniel Axtens Reviewed-by: Daniel Kiper --- grub-core/kern/file.c | 2 ++

[SECURITY PATCH 16/30] net/ip: Do IP fragment maths safely

2022-06-07 Thread Daniel Kiper
From: Daniel Axtens We can receive packets with invalid IP fragmentation information. This can lead to rsm->total_len underflowing and becoming very large. Then, in grub_netbuff_alloc(), we add to this very large number, which can cause it to overflow and wrap back around to a small positive num

[SECURITY PATCH 03/30] loader/efi/chainloader: Use grub_loader_set_ex()

2022-06-07 Thread Daniel Kiper
From: Chris Coulson This ports the EFI chainloader to use grub_loader_set_ex() in order to fix a use-after-free bug that occurs when grub_cmd_chainloader() is executed more than once before a boot attempt is performed. Fixes: CVE-2022-28736 Signed-off-by: Chris Coulson Reviewed-by: Daniel Kipe

[SECURITY PATCH 11/30] video/readers/jpeg: Abort sooner if a read operation fails

2022-06-07 Thread Daniel Kiper
From: Daniel Axtens Fuzzing revealed some inputs that were taking a long time, potentially forever, because they did not bail quickly upon encountering an I/O error. Try to catch I/O errors sooner and bail out. Signed-off-by: Daniel Axtens Reviewed-by: Daniel Kiper --- grub-core/video/reader

[SECURITY PATCH 21/30] net/tftp: Avoid a trivial UAF

2022-06-07 Thread Daniel Kiper
From: Daniel Axtens Under tftp errors, we print a tftp error message from the tftp header. However, the tftph pointer is a pointer inside nb, the netbuff. Previously, we were freeing the nb and then dereferencing it. Don't do that, use it and then free it later. This isn't really _bad_ per se, e

[SECURITY PATCH 17/30] net/netbuff: Block overly large netbuff allocs

2022-06-07 Thread Daniel Kiper
From: Daniel Axtens A netbuff shouldn't be too huge. It's bounded by MTU and TCP segment reassembly. If we are asked to create one that is unreasonably big, refuse. This is a hardening measure: if we hit this code, there's a bug somewhere else that we should catch and fix. This commit: - stop

[SECURITY PATCH 19/30] net/dns: Don't read past the end of the string we're checking against

2022-06-07 Thread Daniel Kiper
From: Daniel Axtens I don't really understand what's going on here but fuzzing found a bug where we read past the end of check_with. That's a C string, so use grub_strlen() to make sure we don't overread it. Signed-off-by: Daniel Axtens Reviewed-by: Daniel Kiper --- grub-core/net/dns.c | 19 +

[SECURITY PATCH 25/30] fs/f2fs: Do not read past the end of nat journal entries

2022-06-07 Thread Daniel Kiper
From: Sudhakar Kuppusamy A corrupt f2fs file system could specify a nat journal entry count that is beyond the maximum NAT_JOURNAL_ENTRIES. Check if the specified nat journal entry count before accessing the array, and throw an error if it is too large. Signed-off-by: Sudhakar Kuppusamy Signed

[SECURITY PATCH 14/30] video/readers/jpeg: Block int underflow -> wild pointer write

2022-06-07 Thread Daniel Kiper
From: Daniel Axtens Certain 1 px wide images caused a wild pointer write in grub_jpeg_ycrcb_to_rgb(). This was caused because in grub_jpeg_decode_data(), we have the following loop: for (; data->r1 < nr1 && (!data->dri || rst); data->r1++, data->bitmap_ptr += (vb * data->image_width - hb *

[SECURITY PATCH 10/30] video/readers/png: Sanity check some huffman codes

2022-06-07 Thread Daniel Kiper
From: Daniel Axtens ASAN picked up two OOB global reads: we weren't checking if some code values fit within the cplens or cpdext arrays. Check and throw an error if not. Signed-off-by: Daniel Axtens Reviewed-by: Daniel Kiper --- grub-core/video/readers/png.c | 6 ++ 1 file changed, 6 inse

[SECURITY PATCH 24/30] net/http: Error out on headers with LF without CR

2022-06-07 Thread Daniel Kiper
From: Daniel Axtens In a similar vein to the previous patch, parse_line() would write a NUL byte past the end of the buffer if there was an HTTP header with a LF rather than a CRLF. RFC-2616 says: Many HTTP/1.1 header field values consist of words separated by LWS or special characters. The

[SECURITY PATCH 20/30] net/tftp: Prevent a UAF and double-free from a failed seek

2022-06-07 Thread Daniel Kiper
From: Daniel Axtens A malicious tftp server can cause UAFs and a double free. An attempt to read from a network file is handled by grub_net_fs_read(). If the read is at an offset other than the current offset, grub_net_seek_real() is invoked. In grub_net_seek_real(), if a backwards seek cannot

[SECURITY PATCH 09/30] video/readers/png: Avoid heap OOB R/W inserting huff table items

2022-06-07 Thread Daniel Kiper
From: Daniel Axtens In fuzzing we observed crashes where a code would attempt to be inserted into a huffman table before the start, leading to a set of heap OOB reads and writes as table entries with negative indices were shifted around and the new code written in. Catch the case where we would

[SECURITY PATCH 26/30] fs/f2fs: Do not read past the end of nat bitmap

2022-06-07 Thread Daniel Kiper
From: Sudhakar Kuppusamy A corrupt f2fs filesystem could have a block offset or a bitmap offset that would cause us to read beyond the bounds of the nat bitmap. Introduce the nat_bitmap_size member in grub_f2fs_data which holds the size of nat bitmap. Set the size when loading the nat bitmap in

[SECURITY PATCH 08/30] video/readers/png: Drop greyscale support to fix heap out-of-bounds write

2022-06-07 Thread Daniel Kiper
From: Daniel Axtens A 16-bit greyscale PNG without alpha is processed in the following loop: for (i = 0; i < (data->image_width * data->image_height); i++, d1 += 4, d2 += 2) { d1[R3] = d2[1]; d1[G3] = d2[1]; d1[B3] = d2[1]; } The in

[SECURITY PATCH 27/30] fs/f2fs: Do not copy file names that are too long

2022-06-07 Thread Daniel Kiper
From: Sudhakar Kuppusamy A corrupt f2fs file system might specify a name length which is greater than the maximum name length supported by the GRUB f2fs driver. We will allocate enough memory to store the overly long name, but there are only F2FS_NAME_LEN bytes in the source, so we would read pa

[SECURITY PATCH 06/30] video/readers/png: Abort sooner if a read operation fails

2022-06-07 Thread Daniel Kiper
From: Daniel Axtens Fuzzing revealed some inputs that were taking a long time, potentially forever, because they did not bail quickly upon encountering an I/O error. Try to catch I/O errors sooner and bail out. Signed-off-by: Daniel Axtens Reviewed-by: Daniel Kiper --- grub-core/video/reader

[SECURITY PATCH 07/30] video/readers/png: Refuse to handle multiple image headers

2022-06-07 Thread Daniel Kiper
From: Daniel Axtens This causes the bitmap to be leaked. Do not permit multiple image headers. Signed-off-by: Daniel Axtens Reviewed-by: Daniel Kiper --- grub-core/video/readers/png.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/grub-core/video/readers/png.c b/grub-core/video/reader

[SECURITY PATCH 29/30] fs/btrfs: Fix more ASAN and SEGV issues found with fuzzing

2022-06-07 Thread Daniel Kiper
From: Darren Kenny The fuzzer is generating btrfs file systems that have chunks with invalid combinations of stripes and substripes for the given RAID configurations. After examining the Linux kernel fs/btrfs/tree-checker.c code, it appears that sub-stripes should only be applied to RAID10, and

[SECURITY PATCH 18/30] net/dns: Fix double-free addresses on corrupt DNS response

2022-06-07 Thread Daniel Kiper
From: Daniel Axtens grub_net_dns_lookup() takes as inputs a pointer to an array of addresses ("addresses") for the given name, and pointer to a number of addresses ("naddresses"). grub_net_dns_lookup() is responsible for allocating "addresses", and the caller is responsible for freeing it if "nad

[SECURITY PATCH 28/30] fs/btrfs: Fix several fuzz issues with invalid dir item sizing

2022-06-07 Thread Daniel Kiper
From: Darren Kenny According to the btrfs code in Linux, the structure of a directory item leaf should be of the form: |struct btrfs_dir_item|name|data| in GRUB the name len and data len are in the grub_btrfs_dir_item structure's n and m fields respectively. The combined size of the structur

[SECURITY PATCH 22/30] net/http: Do not tear down socket if it's already been torn down

2022-06-07 Thread Daniel Kiper
From: Daniel Axtens It's possible for data->sock to get torn down in tcp error handling. If we unconditionally tear it down again we will end up doing writes to an offset of the NULL pointer when we go to tear it down again. Detect if it has been torn down and don't do it again. Signed-off-by:

[SECURITY PATCH 30/30] fs/btrfs: Fix more fuzz issues related to chunks

2022-06-07 Thread Daniel Kiper
From: Darren Kenny The corpus was generating issues in grub_btrfs_read_logical() when attempting to iterate over stripe entries in the superblock's bootmapping. In most cases the reason for the failure was that the number of stripes in chunk->nstripes exceeded the possible space statically alloc

[SECURITY PATCH 23/30] net/http: Fix OOB write for split http headers

2022-06-07 Thread Daniel Kiper
From: Daniel Axtens GRUB has special code for handling an http header that is split across two packets. The code tracks the end of line by looking for a "\n" byte. The code for split headers has always advanced the pointer just past the end of the line, whereas the code that handles unsplit head