[PATCH 2/4] libfsimage/reiserfs: validate item counts and journal length

[SounLabs <holyting07@xxxxxxxxx>]

libfsimage is used by pygrub to parse the filesystem of an untrusted guest
disk image from dom0.  Three reiserfs on-disk counts are used without
validation:

 - block_head.blk_nr_item is iterated as item headers / keys in search_stat()
   and next_key() without checking they fit in the block (heap OOB read).
 - item_head.ih_entry_count is iterated as directory headers in reiserfs_dir()
   without bounding it to the item body (heap OOB read).
 - the journal descriptor/commit j_len indexes commit.j_realblock[], which
   holds JOURNAL_TRANS_HALF entries, in journal_init() and block_read()
   without a bound (stack OOB read).

Check each count against the structure that holds it: blk_nr_item against the
block size when a node is read (including the root), ih_entry_count against
ih_item_len, and j_len against 2 * JOURNAL_TRANS_HALF.

Same class of issue as XSA-497 (iso9660).  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/reiserfs/fsys_reiserfs.c | 46 ++++++++++++++++++++++-
 1 file changed, 45 insertions(+), 1 deletion(-)

diff --git a/tools/libfsimage/reiserfs/fsys_reiserfs.c 
b/tools/libfsimage/reiserfs/fsys_reiserfs.c
index 10ca657476..395488245b 100644
--- a/tools/libfsimage/reiserfs/fsys_reiserfs.c
+++ b/tools/libfsimage/reiserfs/fsys_reiserfs.c
@@ -420,6 +420,13 @@ block_read (fsi_file_t *ffi, int blockNr, int start, int 
len, char *buffer)
            return 0;
 
          j_len = desc.j_len;
+         /* See journal_init(): j_len must not index commit.j_realblock[]
+          * (JOURNAL_TRANS_HALF entries) out of bounds. */
+         if (j_len > JOURNAL_TRANS_HALF * 2)
+           {
+             errnum = ERR_FSYS_CORRUPT;
+             return 0;
+           }
          while (i < j_len && i < JOURNAL_TRANS_HALF)
            if (desc.j_realblock[i++] == blockNr)
              goto found;
@@ -497,7 +504,11 @@ journal_init (fsi_file_t *ffi)
       commit_block = (desc_block + desc.j_len + 1) & (block_count - 1);
       journal_read (ffi, commit_block, sizeof (commit), (char *) &commit);
       if (desc.j_trans_id != commit.j_trans_id
-         || desc.j_len != commit.j_len)
+         || desc.j_len != commit.j_len
+         /* j_len indexes desc.j_realblock[] and commit.j_realblock[],
+          * which together hold at most 2 * JOURNAL_TRANS_HALF entries;
+          * a larger (attacker-controlled) value reads past 'commit'. */
+         || desc.j_len > JOURNAL_TRANS_HALF * 2)
        /* no more valid transactions */
        break;
       
@@ -555,6 +566,8 @@ journal_init (fsi_file_t *ffi)
   return errnum == 0;
 }
 
+static int blkh_nr_item_ok (char *cache, unsigned int blocksize);
+
 /* check filesystem types and read superblock into memory buffer */
 static int
 reiserfs_mount (fsi_file_t *ffi, const char *options)
@@ -646,6 +659,8 @@ reiserfs_mount (fsi_file_t *ffi, const char *options)
 
   if (INFO->tree_depth >= MAX_HEIGHT)
     return 0;
+  if (! blkh_nr_item_ok (ROOT, INFO->blocksize))
+    return 0;
   if (INFO->tree_depth == DISK_LEAF_NODE_LEVEL)
     {
       /* There is only one node in the whole filesystem, 
@@ -676,6 +691,22 @@ reiserfs_mount (fsi_file_t *ffi, const char *options)
  * efficient, but this really doesn't hurt for grub.  
  */
 
+/* A block_head's blk_nr_item is attacker-controlled.  Confirm the keys or
+ * item headers it claims actually fit inside the on-disk block, so the
+ * item/key scans in search_stat() and next_key() cannot run past the
+ * cached block. */
+static int
+blkh_nr_item_ok (char *cache, unsigned int blocksize)
+{
+  unsigned int nr = BLOCKHEAD (cache)->blk_nr_item;
+  if (BLOCKHEAD (cache)->blk_level == DISK_LEAF_NODE_LEVEL)
+    /* leaf: block_head followed by nr_item item headers */
+    return (unsigned long) nr * IH_SIZE + BLKH_SIZE <= blocksize;
+  /* internal: block_head, nr_item keys, nr_item + 1 disk-child pointers */
+  return (unsigned long) nr * (KEY_SIZE + DC_SIZE) + DC_SIZE + BLKH_SIZE
+        <= blocksize;
+}
+
 /* Read in the node at the current path and depth into the node cache.
  * You must set INFO->blocks[depth] before.
  */
@@ -707,6 +738,11 @@ read_tree_node (fsi_file_t *ffi, unsigned int blockNr, int 
depth)
       errnum = ERR_FSYS_CORRUPT;
       return 0;
     }
+  if (! blkh_nr_item_ok (cache, INFO->blocksize))
+    {
+      errnum = ERR_FSYS_CORRUPT;
+      return 0;
+    }
 
   INFO->blocks[depth] = blockNr;
   return cache;
@@ -1140,6 +1176,14 @@ reiserfs_dir (fsi_file_t *ffi, char *dirname)
          name_end = INFO->current_item + INFO->current_ih->ih_item_len;
          de_head = (struct reiserfs_de_head *) INFO->current_item;
          num_entries = INFO->current_ih->u.ih_entry_count;
+         /* ih_entry_count is attacker-controlled; the directory headers it
+          * counts (DEH_SIZE each) must fit within this item's body. */
+         if ((unsigned long) num_entries * DEH_SIZE
+             > INFO->current_ih->ih_item_len)
+           {
+             errnum = ERR_FSYS_CORRUPT;
+             return 0;
+           }
          while (num_entries > 0)
            {
              char *filename = INFO->current_item + de_head->deh_location;
-- 
2.52.0.windows.1