[PATCH 3/4] libfsimage/ufs: validate superblock divisors and directory record length

[SounLabs <holyting07@xxxxxxxxx>]

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

 - fs_inopb and fs_ipg are used as divisors by the itoo()/itog()/itod()
   macros in openi(); a zero value divides by zero (SIGFPE).
 - a directory entry's d_reclen advances the scan in dlook(); a zero value
   never advances and spins forever.

Reject a non-positive fs_inopb or fs_ipg at mount time, and stop the
directory scan on a zero d_reclen.

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/ufs/fsys_ufs.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/tools/libfsimage/ufs/fsys_ufs.c b/tools/libfsimage/ufs/fsys_ufs.c
index a545bc9f65..12cc2f700d 100644
--- a/tools/libfsimage/ufs/fsys_ufs.c
+++ b/tools/libfsimage/ufs/fsys_ufs.c
@@ -49,7 +49,12 @@ ufs_mount(fsi_file_t *ffi, const char *options)
        if (/*! IS_PC_SLICE_TYPE_SOLARIS(current_slice) || */
            !devread(ffi, UFS_SBLOCK, 0, UFS_SBSIZE, (char *)SUPERBLOCK) ||
            SUPERBLOCK->fs_magic != UFS_MAGIC ||
-           MAXBSIZE < SUPERBLOCK->fs_bsize)
+           MAXBSIZE < SUPERBLOCK->fs_bsize ||
+           /* fs_inopb and fs_ipg are attacker-controlled and are used as
+            * divisors by the itoo()/itog()/itod() macros in openi(); a
+            * zero (or negative) value divides by zero (SIGFPE). */
+           SUPERBLOCK->fs_inopb <= 0 ||
+           SUPERBLOCK->fs_ipg <= 0)
                return 0;
 
        return 1;
@@ -257,6 +262,10 @@ dlook(fsi_file_t *ffi, grub_ino_t dir_ino, char *name)
                dp = (struct direct *)(DIRENT + off);
                if (dp->d_ino && substring(name, dp->d_name) == 0)
                        return (dp->d_ino);
+               /* d_reclen is attacker-controlled; a zero value would never
+                * advance loc and would spin forever. */
+               if (dp->d_reclen == 0)
+                       return (0);
                loc += dp->d_reclen;
        }
        return (0);
-- 
2.52.0.windows.1