grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH 1/2] gzio: abort early when get_byte reads nothing


From: Daniel Axtens
Subject: [PATCH 1/2] gzio: abort early when get_byte reads nothing
Date: Mon, 13 May 2024 00:32:08 +1000

This isn't intended to be a functional change, but it makes a lot of failures a 
lot
faster, which is extremely helpful for fuzzing.

Without this change, we keep trying and trying to read more bytes into our 
buffer,
never being able to (read always returns 0) and so we just return old buffer 
contents
over and over until the decompression process fails some other way.
---
 grub-core/io/gzio.c | 53 +++++++++++++++++++++++++++++----------------
 1 file changed, 34 insertions(+), 19 deletions(-)

diff --git a/grub-core/io/gzio.c b/grub-core/io/gzio.c
index 71b2132dc..8f23a1705 100644
--- a/grub-core/io/gzio.c
+++ b/grub-core/io/gzio.c
@@ -366,6 +366,10 @@ static int dbits = 6;              /* bits in base 
distance lookup table */
    lookup of seven bits, the EOB code will be found in that first
    lookup, and so will not require that too many bits be pulled from
    the stream.
+
+   Note that NEEDBITS() can fail if the file is corrupt/truncated.
+   The GOTOFAILIFERROR, RETURNIFERROR and RETURN1IFERROR macros can
+   help.
  */
 
 static ush mask_bits[] =
@@ -377,10 +381,14 @@ static ush mask_bits[] =
 
 #define NEEDBITS(n) do {while(k<(n)){b|=((ulg)get_byte(gzio))<<k;k+=8;}} while 
(0)
 #define DUMPBITS(n) do {b>>=(n);k-=(n);} while (0)
+#define RETURNIFERROR if (grub_errno != GRUB_ERR_NONE) return
+#define RETURN1IFERROR if (grub_errno != GRUB_ERR_NONE) return 1
+#define GOTOFAILIFERROR if (grub_errno != GRUB_ERR_NONE) goto fail
 
 static int
 get_byte (grub_gzio_t gzio)
 {
+  grub_ssize_t bytes_read;
   if (gzio->mem_input)
     {
       if (gzio->mem_input_off < gzio->mem_input_size)
@@ -393,7 +401,14 @@ get_byte (grub_gzio_t gzio)
                     || gzio->inbuf_d == INBUFSIZ))
     {
       gzio->inbuf_d = 0;
-      grub_file_read (gzio->file, gzio->inbuf, INBUFSIZ);
+      bytes_read = grub_file_read (gzio->file, gzio->inbuf, INBUFSIZ);
+      /* Only trigger if we get 0 bytes: if we get negative bytes there should
+       * be an existing grub error code that we don't want to overwrite. */
+      if (bytes_read == 0)
+       {
+         grub_error (GRUB_ERR_BAD_COMPRESSED_DATA, "File is too short");
+         return 0;
+       }
     }
 
   return gzio->inbuf[gzio->inbuf_d++];
@@ -684,7 +699,7 @@ inflate_codes_in_window (grub_gzio_t gzio)
              return 1;
            }
 
-         NEEDBITS ((unsigned) gzio->bl);
+         NEEDBITS ((unsigned) gzio->bl); RETURN1IFERROR;
          if ((e = (t = gzio->tl + ((unsigned) b & ml))->e) > 16)
            do
              {
@@ -696,7 +711,7 @@ inflate_codes_in_window (grub_gzio_t gzio)
                  }
                DUMPBITS (t->b);
                e -= 16;
-               NEEDBITS (e);
+               NEEDBITS (e); RETURN1IFERROR;
              }
            while ((e = (t = t->v.t + ((unsigned) b & mask_bits[e]))->e) > 16);
          DUMPBITS (t->b);
@@ -718,7 +733,7 @@ inflate_codes_in_window (grub_gzio_t gzio)
                }
 
              /* get length of block to copy */
-             NEEDBITS (e);
+             NEEDBITS (e); RETURN1IFERROR;
              n = t->v.n + ((unsigned) b & mask_bits[e]);
              DUMPBITS (e);
 
@@ -729,7 +744,7 @@ inflate_codes_in_window (grub_gzio_t gzio)
                }
 
              /* decode distance of block to copy */
-             NEEDBITS ((unsigned) gzio->bd);
+             NEEDBITS ((unsigned) gzio->bd); RETURN1IFERROR;
              if ((e = (t = gzio->td + ((unsigned) b & md))->e) > 16)
                do
                  {
@@ -741,12 +756,12 @@ inflate_codes_in_window (grub_gzio_t gzio)
                      }
                    DUMPBITS (t->b);
                    e -= 16;
-                   NEEDBITS (e);
+                   NEEDBITS (e); RETURN1IFERROR;
                  }
                while ((e = (t = t->v.t + ((unsigned) b & mask_bits[e]))->e)
                       > 16);
              DUMPBITS (t->b);
-             NEEDBITS (e);
+             NEEDBITS (e); RETURN1IFERROR;
              d = w - t->v.n - ((unsigned) b & mask_bits[e]);
              DUMPBITS (e);
              gzio->code_state++;
@@ -815,10 +830,10 @@ init_stored_block (grub_gzio_t gzio)
   DUMPBITS (k & 7);
 
   /* get the length and its complement */
-  NEEDBITS (16);
+  NEEDBITS (16); RETURNIFERROR;
   gzio->block_len = ((unsigned) b & 0xffff);
   DUMPBITS (16);
-  NEEDBITS (16);
+  NEEDBITS (16); RETURNIFERROR;
   if (gzio->block_len != (int) ((~b) & 0xffff))
     grub_error (GRUB_ERR_BAD_COMPRESSED_DATA,
                "the length of a stored block does not match");
@@ -900,13 +915,13 @@ init_dynamic_block (grub_gzio_t gzio)
   k = gzio->bk;
 
   /* read in table lengths */
-  NEEDBITS (5);
+  NEEDBITS (5); RETURNIFERROR;
   nl = 257 + ((unsigned) b & 0x1f);    /* number of literal/length codes */
   DUMPBITS (5);
-  NEEDBITS (5);
+  NEEDBITS (5); RETURNIFERROR;
   nd = 1 + ((unsigned) b & 0x1f);      /* number of distance codes */
   DUMPBITS (5);
-  NEEDBITS (4);
+  NEEDBITS (4); RETURNIFERROR;
   nb = 4 + ((unsigned) b & 0xf);       /* number of bit length codes */
   DUMPBITS (4);
   if (nl > 286 || nd > 30)
@@ -918,7 +933,7 @@ init_dynamic_block (grub_gzio_t gzio)
   /* read in bit-length-code lengths */
   for (j = 0; j < nb; j++)
     {
-      NEEDBITS (3);
+      NEEDBITS (3); RETURNIFERROR;
       ll[bitorder[j]] = (unsigned) b & 7;
       DUMPBITS (3);
     }
@@ -947,7 +962,7 @@ init_dynamic_block (grub_gzio_t gzio)
 
   while ((unsigned) i < n)
     {
-      NEEDBITS ((unsigned) gzio->bl);
+      NEEDBITS ((unsigned) gzio->bl); GOTOFAILIFERROR;
       j = (gzio->td = gzio->tl + ((unsigned) b & m))->b;
       DUMPBITS (j);
       j = gzio->td->v.n;
@@ -955,7 +970,7 @@ init_dynamic_block (grub_gzio_t gzio)
        ll[i++] = l = j;        /* save last length in l */
       else if (j == 16)                /* repeat last length 3 to 6 times */
        {
-         NEEDBITS (2);
+         NEEDBITS (2); GOTOFAILIFERROR;
          j = 3 + ((unsigned) b & 3);
          DUMPBITS (2);
          if ((unsigned) i + j > n)
@@ -968,7 +983,7 @@ init_dynamic_block (grub_gzio_t gzio)
        }
       else if (j == 17)                /* 3 to 10 zero length codes */
        {
-         NEEDBITS (3);
+         NEEDBITS (3); GOTOFAILIFERROR;
          j = 3 + ((unsigned) b & 7);
          DUMPBITS (3);
          if ((unsigned) i + j > n)
@@ -983,7 +998,7 @@ init_dynamic_block (grub_gzio_t gzio)
       else
        /* j == 18: 11 to 138 zero length codes */
        {
-         NEEDBITS (7);
+         NEEDBITS (7); GOTOFAILIFERROR;
          j = 11 + ((unsigned) b & 0x7f);
          DUMPBITS (7);
          if ((unsigned) i + j > n)
@@ -1049,12 +1064,12 @@ get_new_block (grub_gzio_t gzio)
   k = gzio->bk;
 
   /* read in last block bit */
-  NEEDBITS (1);
+  NEEDBITS (1); RETURNIFERROR;
   gzio->last_block = (int) b & 1;
   DUMPBITS (1);
 
   /* read in block type */
-  NEEDBITS (2);
+  NEEDBITS (2); RETURNIFERROR;
   gzio->block_type = (unsigned) b & 3;
   DUMPBITS (2);
 
-- 
2.39.3 (Apple Git-145)




reply via email to

[Prev in Thread] Current Thread [Next in Thread]