GRUB 2.12 release - update

2022-10-26 Thread Daniel Kiper
Hi,

We are getting closer to the 2.12 release. Sadly we still do not have
many of important patch sets in the tree. So, I am going to spend more
time on reviews in the following weeks. Below you can find my list of
key patch sets which should land in the release:
  - Dynamic allocation of memory regions and IBM vTPM v2,
  - Unify ARM64 & RISC-V Linux Loader,
  - Add support for LoongArch,
  - plainmount: Support plain encryption mode,
  - Glenn's tests fixes.

Of course all patches which got my RB or are under review will be merged too.

There is also "nice to have" list but I do not consider lack of this
patch sets as release blockers:
  - Add support for EFI file system transposition,
  - term/serial: Add support for PCI serial devices,
  - Add MMIO serial ports support and SPCR detection,
  - Glenn's gdb patch set.

I hope I will be able to review and merge all patch sets from first list
during November. Then if everything goes well we will make code freeze
in December and release at the beginning of next year, preferably in January.

I am considering to not block merges for tests and documentation during
code freeze.

If you have any comments on that plan please drop me a line.

Daniel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [External] : Re: [PATCH 2/2] video/readers/jpeg: Check next_marker is within file size

2022-10-26 Thread Alec Brown
On Sat, Oct 22, 2022 at 12:52:02AM +1100, Daniel Axtens wrote:
> Alec Brown  writes:
> 
> > In grub-core/video/readers/jpeg.c, the function 
> > grub_jpeg_decode_huff_table()
> > has the variable next_marker which reads data from grub_jpeg_get_word() and
> > then uses it as an upper limit in a while loop. However, the function isn't
> > checking that next_marker is within the file size, so this check is being 
> > added
> > to the function.
> >
> > Signed-off-by: Alec Brown 
> > ---
> >  grub-core/video/readers/jpeg.c | 6 ++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/grub-core/video/readers/jpeg.c b/grub-core/video/readers/jpeg.c
> > index 0eeea0e63..c0f95fbf9 100644
> > --- a/grub-core/video/readers/jpeg.c
> > +++ b/grub-core/video/readers/jpeg.c
> > @@ -199,6 +199,12 @@ grub_jpeg_decode_huff_table (struct grub_jpeg_data 
> > *data)
> >next_marker = data->file->offset;
> >next_marker += grub_jpeg_get_word (data);
> >  
> > +  if (next_marker > data->file->size)
> > +{
> > +  /* Should never be set beyond the size of the file. */
> > +  return grub_error (GRUB_ERR_BAD_FILE_TYPE, "jpeg: invalid next 
> > reference");
> > +}
> > +
> 
> This is a good idea. I briefly wondered why this didn't lead to issues
> detected in fuzzing. It'll be because the code reads byte by byte until
> we get an EOF and then the loop will bail. Your change is still good, I
> just wanted to double check that you hadn't accidentally found and fixed
> a more serious bug.
>

I took a look at the code again and realized I made a mistake. There is already
a fix for this issue. Turns out the branch I was using to develop my code was
outdated and this patch is redundant. My bad.

Alec Brown

> The fuller context:
> 
>   while (data->file->offset + sizeof (count) + 1 <= next_marker)
> {
>   id = grub_jpeg_get_byte (data);
>   if (grub_errno != GRUB_ERR_NONE)
>   return grub_errno;
> 
> 
> >while (data->file->offset + sizeof (count) + 1 <= next_marker)
> >  {
> >id = grub_jpeg_get_byte (data);
> > -- 
> > 2.27.0
> >
> >
> > ___
> > Grub-devel mailing list
> > Grub-devel@gnu.org
> > https://urldefense.com/v3/__https://lists.gnu.org/mailman/listinfo/grub-devel__;!!ACWV5N9M2RV99hQ!IlmVbvmGcbJZqlfowZKaM9HBO2MzpJDPrCjfmlLWmsN8tlaAmt5iNq5JZTiU3rQor7YXKdCwTYpAAQ$
> >   

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH v2] video/readers: Add artificial limit to image dimensions

2022-10-26 Thread Alec Brown
In grub-core/video/readers/jpeg.c, the height and width of a JPEG image don't
have an upper limit for how big the JPEG image can be. In coverity, this is
getting flagged as an untrusted loop bound. This issue can also seen in PNG and
TGA format images as well but coverity isn't flagging it. To prevent this, the
constant IMAGE_HW_MAX_PX is being added to bitmap.h, which has a value of 16384,
to act as an artifical limit and restrict the height and width of images. This
value was picked as it is double the current max resolution size, which is 8K.

Fixes: CID 292450

Signed-off-by: Alec Brown 
---

In v1, the patch set was developed on outdated code and there was
already a fix for the second patch. So in this version, the second patch
has been dropped. The only thing that has changed in this patch is line
numbers.

 docs/grub.texi | 3 ++-
 grub-core/video/readers/jpeg.c | 6 +-
 grub-core/video/readers/png.c  | 6 +-
 grub-core/video/readers/tga.c  | 7 +++
 include/grub/bitmap.h  | 2 ++
 5 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/docs/grub.texi b/docs/grub.texi
index 0dbbdc374..2d6cd8358 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -1515,7 +1515,8 @@ resolution.  @xref{gfxmode}.
 Set a background image for use with the @samp{gfxterm} graphical terminal.
 The value of this option must be a file readable by GRUB at boot time, and
 it must end with @file{.png}, @file{.tga}, @file{.jpg}, or @file{.jpeg}.
-The image will be scaled if necessary to fit the screen.
+The image will be scaled if necessary to fit the screen. Image height and
+width will be restricted by an artificial limit of 16384.
 
 @item GRUB_THEME
 Set a theme for use with the @samp{gfxterm} graphical terminal.
diff --git a/grub-core/video/readers/jpeg.c b/grub-core/video/readers/jpeg.c
index 09596fbf5..ae634fd41 100644
--- a/grub-core/video/readers/jpeg.c
+++ b/grub-core/video/readers/jpeg.c
@@ -346,7 +346,11 @@ grub_jpeg_decode_sof (struct grub_jpeg_data *data)
   data->image_height = grub_jpeg_get_word (data);
   data->image_width = grub_jpeg_get_word (data);
 
-  if ((!data->image_height) || (!data->image_width))
+  grub_dprintf ("jpeg", "image height: %d\n", data->image_height);
+  grub_dprintf ("jpeg", "image width: %d\n", data->image_width);
+
+  if ((!data->image_height) || (!data->image_width) ||
+  (data->image_height > IMAGE_HW_MAX_PX) || (data->image_width > 
IMAGE_HW_MAX_PX))
 return grub_error (GRUB_ERR_BAD_FILE_TYPE, "jpeg: invalid image size");
 
   cc = grub_jpeg_get_byte (data);
diff --git a/grub-core/video/readers/png.c b/grub-core/video/readers/png.c
index 7f2ba7849..3163e97bf 100644
--- a/grub-core/video/readers/png.c
+++ b/grub-core/video/readers/png.c
@@ -264,7 +264,11 @@ grub_png_decode_image_header (struct grub_png_data *data)
   data->image_width = grub_png_get_dword (data);
   data->image_height = grub_png_get_dword (data);
 
-  if ((!data->image_height) || (!data->image_width))
+  grub_dprintf ("png", "image height: %d\n", data->image_height);
+  grub_dprintf ("png", "image width: %d\n", data->image_width);
+
+  if ((!data->image_height) || (!data->image_width) ||
+  (data->image_height > IMAGE_HW_MAX_PX) || (data->image_width > 
IMAGE_HW_MAX_PX))
 return grub_error (GRUB_ERR_BAD_FILE_TYPE, "png: invalid image size");
 
   color_bits = grub_png_get_byte (data);
diff --git a/grub-core/video/readers/tga.c b/grub-core/video/readers/tga.c
index a9ec3a1b6..f2f563d06 100644
--- a/grub-core/video/readers/tga.c
+++ b/grub-core/video/readers/tga.c
@@ -340,6 +340,13 @@ grub_video_reader_tga (struct grub_video_bitmap **bitmap,
   data.image_width = grub_le_to_cpu16 (data.hdr.image_width);
   data.image_height = grub_le_to_cpu16 (data.hdr.image_height);
 
+  grub_dprintf ("tga", "image height: %d\n", data.image_height);
+  grub_dprintf ("tga", "image width: %d\n", data.image_width);
+
+  /* Check image height and width are within restrictions */
+  if ((data.image_height > IMAGE_HW_MAX_PX) || (data.image_width > 
IMAGE_HW_MAX_PX))
+return grub_error (GRUB_ERR_BAD_FILE_TYPE, "tga: invalid image size");
+
   /* Check that bitmap encoding is supported.  */
   switch (data.hdr.image_type)
 {
diff --git a/include/grub/bitmap.h b/include/grub/bitmap.h
index 5728f8ca3..149d37bfe 100644
--- a/include/grub/bitmap.h
+++ b/include/grub/bitmap.h
@@ -24,6 +24,8 @@
 #include 
 #include 
 
+#define IMAGE_HW_MAX_PX16384
+
 struct grub_video_bitmap
 {
   /* Bitmap format description.  */
-- 
2.27.0


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel