io/gzio: Abort early when get_byte() reads nothing

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.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

diff --git a/grub-core/io/gzio.c b/grub-core/io/gzio.c
index ca935575135667b6feeeb46aca63085e1ad73a85..b882b559a1c5e069de4d4ced285070c1685a67c7 100644 (file)
--- 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,16 @@ 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 +701,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 +713,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 +735,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 +746,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 +758,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 +832,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 +917,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 +935,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 +964,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 +972,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 +985,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 +1000,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 +1066,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);