[PATCH 4/4] libfsimage/xfs: bound inline extent count and directory btree traversals

[SounLabs <holyting07@xxxxxxxxx>]

libfsimage is used by pygrub to parse the filesystem of an untrusted guest
disk image from dom0.  Several xfs on-disk fields are used without validation,
leading to an out-of-bounds read and to unbounded tree descents that loop
forever:

 - init_extents(), XFS_DINODE_FMT_EXTENTS: di_nextents is taken as the number
   of inline extent records; next_extent() then iterates that many records from
   the inode data fork without checking they fit (heap OOB read).

 - init_extents(), XFS_DINODE_FMT_BTREE: the root-to-leaf descent follows
   bb_level/ptr0 with no depth limit, so a node whose bb_level does not
   decrease (e.g. one pointing to itself) loops forever.

 - first_dentry(), directory da-btree: the root-to-leaf descent assigns
   xfs.dablk = btree[0].before with no depth limit or cycle detection, so a
   self-referential or cyclic directory node loops forever.  This is the loop
   reached by the di_nextents reproducer as well, which therefore triggers
   both the OOB read above and this unbounded da-descent hang.  When the
   descent is bounded and the directory yields no entries, first_dentry()
   returns NULL and the caller in xfs_dir() dereferenced that NULL name via
   substring(dirname, NULL) (NULL read).

Cap di_nextents to the number of records that fit in the inode data fork,
bound both the bmap btree and the directory da-btree descents to a fixed
maximum depth, and have xfs_dir() fail the lookup gracefully when a directory
yields no first entry.

Same class of issue as XSA-497 (iso9660); the superblock sanity check added
by XSA-443 does not cover these inode-level and btree-level fields.  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/xfs/fsys_xfs.c | 57 +++++++++++++++++++++++++++++++--
 1 file changed, 54 insertions(+), 3 deletions(-)

diff --git a/tools/libfsimage/xfs/fsys_xfs.c b/tools/libfsimage/xfs/fsys_xfs.c
index e98b367901..7fbd78a122 100644
--- a/tools/libfsimage/xfs/fsys_xfs.c
+++ b/tools/libfsimage/xfs/fsys_xfs.c
@@ -201,20 +201,42 @@ di_read (fsi_file_t *ffi, xfs_ino_t ino)
        return 1;
 }
 
+/* An XFS bmap btree is only a handful of levels deep; use a generous cap
+ * to bound a traversal that would otherwise loop forever on a node whose
+ * bb_level does not decrease (e.g. one that points to itself). */
+#define XFS_BM_MAXLEVELS 32
+
+/* A directory da-btree is likewise only a few levels deep; bound the
+ * root-to-leaf descent in first_dentry() so a node whose btree[0].before
+ * never reaches a leaf (e.g. one that points to itself) cannot loop forever. 
*/
+#define XFS_DA_MAXLEVELS 32
+
 static void
 init_extents (fsi_file_t *ffi)
 {
        xfs_bmbt_ptr_t ptr0;
        xfs_btree_lblock_t h;
+       int levels;
 
        switch (icore.di_format) {
-       case XFS_DINODE_FMT_EXTENTS:
+       case XFS_DINODE_FMT_EXTENTS: {
+               /* di_nextents is attacker-controlled; the inline extent
+                * records (xfs_bmbt_rec_32_t each) must fit in the inode's
+                * data fork, or next_extent() reads past the inode buffer. */
+               int fork_bytes =
+                   (icore.di_forkoff ? (icore.di_forkoff << 3) : xfs.isize)
+                   - (int) offsetof(xfs_dinode_t, di_u);
+               unsigned int maxext = fork_bytes > 0
+                   ? (unsigned int) fork_bytes / sizeof(xfs_bmbt_rec_32_t) : 0;
                xfs.xt = inode->di_u.di_bmx;
                xfs.nextents = le32 (icore.di_nextents);
+               if (xfs.nextents > maxext)
+                       xfs.nextents = maxext;
                break;
+       }
        case XFS_DINODE_FMT_BTREE:
                ptr0 = xfs.ptr0;
-               for (;;) {
+               for (levels = 0; ; levels++) {
                        xfs.daddr = fsb2daddr (le64(ptr0));
                        devread (ffi, xfs.daddr, 0,
                                 sizeof(xfs_btree_lblock_t), (char *)&h);
@@ -224,6 +246,12 @@ init_extents (fsi_file_t *ffi)
                                xfs.fpos = sizeof(xfs_btree_block_t);
                                return;
                        }
+                       if (levels >= XFS_BM_MAXLEVELS) {
+                               /* malformed btree: refuse to iterate */
+                               xfs.nextents = 0;
+                               xfs.next = 0;
+                               return;
+                       }
                        devread (ffi, xfs.daddr, xfs.btnode_ptr0_off,
                                 sizeof(xfs_bmbt_ptr_t), (char *)&ptr0);
                }
@@ -392,6 +420,8 @@ next_dentry (fsi_file_t *ffi, xfs_ino_t *ino)
 static char *
 first_dentry (fsi_file_t *ffi, xfs_ino_t *ino)
 {
+       int da_levels;
+
        xfs.forw = 0;
        switch (icore.di_format) {
        case XFS_DINODE_FMT_LOCAL:
@@ -413,7 +443,7 @@ first_dentry (fsi_file_t *ffi, xfs_ino_t *ino)
                        xfs.dablk = (1ULL << 35) >> xfs.blklog;
 #define h              ((xfs_dir2_leaf_hdr_t *)dirbuf)
 #define n              ((xfs_da_intnode_t *)dirbuf)
-                       for (;;) {
+                       for (da_levels = 0; ; da_levels++) {
                                xfs_dabread (ffi);
                                if ((n->hdr.info.magic == 
le16(XFS_DIR2_LEAFN_MAGIC))
                                    || (n->hdr.info.magic == 
le16(XFS_DIR2_LEAF1_MAGIC))) {
@@ -421,6 +451,16 @@ first_dentry (fsi_file_t *ffi, xfs_ino_t *ino)
                                        xfs.forw = le32 (h->info.forw);
                                        break;
                                }
+                               if (da_levels >= XFS_DA_MAXLEVELS) {
+                                       /* malformed directory da-btree: a node
+                                        * that never descends to a leaf (e.g. a
+                                        * btree[0].before that points to 
itself)
+                                        * would otherwise loop forever.  Treat
+                                        * the directory as empty. */
+                                       xfs.dirmax = 0;
+                                       xfs.forw = 0;
+                                       break;
+                               }
                                xfs.dablk = le32 (n->btree[0].before);
                        }
 #undef n
@@ -602,6 +642,17 @@ xfs_dir (fsi_file_t *ffi, char *dirname)
                *rest = 0;
 
                name = first_dentry (ffi, &new_ino);
+               if (name == NULL) {
+                       /* A malformed directory (e.g. one whose da-btree
+                        * descent was bounded above) yields no entries;
+                        * fail the lookup instead of dereferencing NULL. */
+                       if (print_possibilities < 0)
+                               return 1;
+
+                       errnum = ERR_FILE_NOT_FOUND;
+                       *rest = ch;
+                       return 0;
+               }
                for (;;) {
                        cmp = (!*dirname) ? -1 : substring (dirname, name);
 #ifndef STAGE1_5
-- 
2.52.0.windows.1