[PATCH 1/4] libfsimage/ext2: validate attacker-controlled superblock and extent fields

[SounLabs <holyting07@xxxxxxxxx>]

libfsimage is used by pygrub to parse the filesystem of an untrusted guest
disk image from dom0.  Several ext2/ext4 metadata fields are used without
validation:

 - s_log_block_size sizes the INODE/DATABLOCK1/DATABLOCK2 buffers inside the
   fixed FSYS_BUFLEN region; a large value makes ext2_rdfsb() write a block
   past the buffer (heap out-of-bounds write).
 - s_inode_size and s_inodes_per_group are used as divisors and can be zero
   (SIGFPE).
 - an ext4 extent header's eh_entries is iterated without bounding it to the
   region that holds the entries (heap out-of-bounds read).
 - ext4_recurse_extent_index() has no depth limit, so a self-referencing
   index block recurses until the stack overflows.

Reject the out-of-range superblock values at mount time, and bound both the
extent-entry count (against the inode inline area or block that holds the
header) and the extent-tree recursion depth.

This is the same class of issue addressed for the iso9660 plugin by XSA-497.
pygrub is only security supported when run de-privileged (XSA-508), so this
is hardening rather than a security fix.

Signed-off-by: SounLabs <holyting07@xxxxxxxxx>
---
 tools/libfsimage/ext2fs/fsys_ext2fs.c | 48 ++++++++++++++++++++++++---
 1 file changed, 44 insertions(+), 4 deletions(-)

diff --git a/tools/libfsimage/ext2fs/fsys_ext2fs.c 
b/tools/libfsimage/ext2fs/fsys_ext2fs.c
index 5ed8fce90e..db635b9f4a 100644
--- a/tools/libfsimage/ext2fs/fsys_ext2fs.c
+++ b/tools/libfsimage/ext2fs/fsys_ext2fs.c
@@ -337,6 +337,22 @@ ext2fs_mount (fsi_file_t *ffi, const char *options)
       || SUPERBLOCK->s_magic != EXT2_SUPER_MAGIC)
       retval = 0;
 
+  /* s_log_block_size, s_inode_size and s_inodes_per_group come straight
+   * from the untrusted on-disk superblock.  The buffer-layout macros
+   * (INODE, DATABLOCK1, DATABLOCK2) place three block-sized buffers in
+   * the fixed FSYS_BUFLEN region, and later code divides by the inode
+   * size and by the inodes-per-group count.  Reject values that would
+   * overflow the buffer or divide by zero.  s_log_block_size is tested
+   * first (short-circuit) so EXT2_BLOCK_SIZE() never shifts out of range. */
+  if (retval
+      && (SUPERBLOCK->s_log_block_size > 6
+         || sizeof (struct ext2_super_block) + sizeof (struct ext2_inode)
+              + 3UL * (unsigned long) EXT2_BLOCK_SIZE (SUPERBLOCK) > 
FSYS_BUFLEN
+         || SUPERBLOCK->s_inode_size == 0
+         || SUPERBLOCK->s_inode_size > EXT2_BLOCK_SIZE (SUPERBLOCK)
+         || SUPERBLOCK->s_inodes_per_group == 0))
+    retval = 0;
+
   return retval;
 }
 
@@ -351,14 +367,29 @@ ext2_rdfsb (fsi_file_t *ffi, int fsblock, char *buffer)
                  EXT2_BLOCK_SIZE (SUPERBLOCK), (char *) buffer);
 }
 
-/* Walk through extents index tree to find the good leaf */
+/* ext4 extent trees are at most 5 levels deep in the on-disk format. */
+#define EXT4_MAX_EXTENT_DEPTH 5
+
+/* Walk through extents index tree to find the good leaf.
+ * max_bytes bounds the region the header and its entries occupy: the
+ * inode's 60-byte inline area for the first call, a full block for an
+ * index block read from disk.  This stops a corrupted eh_entries from
+ * being iterated past that region (both ext4_extent and ext4_extent_idx
+ * are the same size).  depth bounds the recursion so a self-referencing
+ * index block cannot spin or overflow the stack. */
 static struct ext4_extent_header *
-ext4_recurse_extent_index(fsi_file_t *ffi, struct ext4_extent_header 
*extent_block, int logical_block)
+ext4_recurse_extent_index(fsi_file_t *ffi, struct ext4_extent_header 
*extent_block, int logical_block, unsigned int max_bytes, int depth)
 {
   int i;
   struct ext4_extent_idx *index = (struct ext4_extent_idx *) (extent_block + 
1);
+  if (depth > EXT4_MAX_EXTENT_DEPTH)
+    return NULL;
   if (extent_block->eh_magic != EXT4_EXT_MAGIC)
     return NULL;
+  if (sizeof (struct ext4_extent_header)
+      + (unsigned int) extent_block->eh_entries * sizeof (struct ext4_extent)
+       > max_bytes)
+    return NULL;
   if (extent_block->eh_depth == 0)
     return extent_block;
   for (i = 0; i < extent_block->eh_entries; i++)
@@ -368,7 +399,7 @@ ext4_recurse_extent_index(fsi_file_t *ffi, struct 
ext4_extent_header *extent_blo
     }
   if (i == 0 || !ext2_rdfsb(ffi, index[i-1].ei_leaf, DATABLOCK1))
     return NULL;
-  return (ext4_recurse_extent_index(ffi, (struct ext4_extent_header *) 
DATABLOCK1, logical_block));
+  return (ext4_recurse_extent_index(ffi, (struct ext4_extent_header *) 
DATABLOCK1, logical_block, EXT2_BLOCK_SIZE (SUPERBLOCK), depth + 1));
 }
 
 
@@ -483,7 +514,7 @@ ext2fs_block_map (fsi_file_t *ffi, int logical_block)
     {
       int i;
       struct ext4_extent_header *extent_hdr =
-         ext4_recurse_extent_index(ffi, (struct ext4_extent_header *) 
INODE->i_block, logical_block);
+         ext4_recurse_extent_index(ffi, (struct ext4_extent_header *) 
INODE->i_block, logical_block, sizeof (INODE->i_block), 0);
       struct ext4_extent *extent = (struct ext4_extent *) (extent_hdr + 1);
       if ( extent_hdr == NULL || extent_hdr->eh_magic != EXT4_EXT_MAGIC)
         {
@@ -863,6 +894,15 @@ ext2fs_dir (fsi_file_t *ffi, char *dirname)
            }
          off = loc & (EXT2_BLOCK_SIZE (SUPERBLOCK) - 1);
          dp = (struct ext2_dir_entry *) (DATABLOCK2 + off);
+         /* rec_len is attacker-controlled.  It must be at least the fixed
+          * 8-byte directory-entry header; a smaller value (in particular 0)
+          * would never advance loc and would spin forever. */
+         if (dp->rec_len < 8)
+           {
+             errnum = ERR_FSYS_CORRUPT;
+             *rest = ch;
+             return 0;
+           }
          /* advance loc prematurely to next on-disk directory entry  */
          loc += dp->rec_len;
 
-- 
2.52.0.windows.1