DragonFlyBSD Kernel Audit
DF-0859 / fix.diff
← back to finding ↓ download raw
diff --git a/sys/vfs/hpfs/hpfs_alsubr.c b/sys/vfs/hpfs/hpfs_alsubr.c
--- a/sys/vfs/hpfs/hpfs_alsubr.c
+++ b/sys/vfs/hpfs/hpfs_alsubr.c
@@ -46,6 +46,27 @@
 #define	AE_DONE		0		/* Nothing to change */
 #define	AE_SPLIT	2		/* Split was done, ranp is valid */
 
+/*
+ * Maximum legitimate ab_busycnt for the in-kernel containers, computed from
+ * the on-disk data-area sizes (sys/vfs/hpfs/hpfs_alsubr.c includes hpfs.h):
+ *   fnode fn_abd[0x60]  = 96 bytes
+ *   alsec as_abd[0x1E0] = 480 bytes
+ * alleaf_t = 12 bytes, alnode_t = 8 bytes (hpfs.h).
+ *
+ * ab_busycnt is taken straight from the on-disk alblk_t and is untrusted.
+ * These caps prevent the bcopy length arithmetic in hpfs_splitalsec,
+ * hpfs_concatalsec and hpfs_alblk2alsec from writing past the destination
+ * alsec's 512-byte buffer-cache buffer (and reading past the source
+ * fnode/alsec data area) when given a corrupt/forged on-disk image
+ * (DF-0859).
+ */
+#define	HPFS_FN_ABD_SIZE	0x60
+#define	HPFS_AS_ABD_SIZE	0x1E0
+#define	HPFS_FN_MAX_LEAF	(HPFS_FN_ABD_SIZE / sizeof(alleaf_t))	/*  8 */
+#define	HPFS_FN_MAX_NODE	(HPFS_FN_ABD_SIZE / sizeof(alnode_t))	/* 12 */
+#define	HPFS_AS_MAX_LEAF	(HPFS_AS_ABD_SIZE / sizeof(alleaf_t))	/* 40 */
+#define	HPFS_AS_MAX_NODE	(HPFS_AS_ABD_SIZE / sizeof(alnode_t))	/* 60 */
+
 int		hpfs_addextentr (struct hpfsmount *, lsn_t, alleaf_t *,
 			         alnode_t *, u_long *);
 int		hpfs_allocalsec (struct hpfsmount *, lsn_t, struct buf **);
@@ -57,6 +78,21 @@
 				  alnode_t *);
 
 /*
+ * Validate ab_busycnt of an on-disk alblk_t against the container maximum.
+ * `is_fnode' selects the fnode (96-byte data area) vs alsec (480-byte data
+ * area) maximum; AB_NODES picks alnode_t (8B) vs alleaf_t (12B) element
+ * size.  Returns 0 if in range, -1 if forged/out-of-range.
+ */
+static int
+hpfs_ab_busycnt_ok(const alblk_t *abp, int is_fnode)
+{
+	u_int maxcnt = (abp->ab_flag & AB_NODES) ?
+	    (is_fnode ? HPFS_FN_MAX_NODE : HPFS_AS_MAX_NODE) :
+	    (is_fnode ? HPFS_FN_MAX_LEAF : HPFS_AS_MAX_LEAF);
+	return (abp->ab_busycnt <= maxcnt) ? 0 : -1;
+}
+
+/*
  * Map file offset to disk offset. hpfsnode have to be locked.
  */
 int
@@ -214,13 +250,22 @@
 	alblk_t *nabp;
 	int error, n1, n2, sz;
 
+	abp = &asp->as_ab;
+
+	/* DF-0859: validate ab_busycnt against alsec container max before
+	 * using it to size the source-offset and length of the bcopy below. */
+	if (hpfs_ab_busycnt_ok(abp, /*is_fnode=*/0) != 0) {
+		kprintf("hpfs_splitalsec: forged ab_busycnt %u > alsec max\n",
+		    abp->ab_busycnt);
+		return (EINVAL);
+	}
+
 	error = hpfs_allocalsec(hpmp, asp->as_parent, &nbp);
 	if (error)
 		return (error);
 
 	nasp = (alsec_t *)nbp->b_data;
 	nabp = &nasp->as_ab;
-	abp = &asp->as_ab;
 
 	n1 = (abp->ab_busycnt + 1) / 2;
 	n2 = (abp->ab_busycnt - n1);
@@ -266,6 +311,19 @@
 
 	ab0p = &as0p->as_ab;
 	ab1p = &as1p->as_ab;
+
+	/* DF-0859: both ab_freecnt and ab_busycnt are untrusted on-disk
+	 * bytes; validate both against the alsec container max before the
+	 * guard and the bcopy use them. */
+	if (hpfs_ab_busycnt_ok(ab0p, /*is_fnode=*/0) != 0 ||
+	    hpfs_ab_busycnt_ok(ab1p, /*is_fnode=*/0) != 0) {
+		kprintf("hpfs_concatalsec: forged ab_busycnt/freecnt "
+		    "(ab0 busycnt=%u freecnt=%u, ab1 busycnt=%u freecnt=%u)\n",
+		    ab0p->ab_busycnt, ab0p->ab_freecnt,
+		    ab1p->ab_busycnt, ab1p->ab_freecnt);
+		return (EINVAL);
+	}
+
 	sz = (ab0p->ab_flag & AB_NODES) ? sizeof(alnode_t) : sizeof(alleaf_t);
 
 	if (ab0p->ab_freecnt > ab1p->ab_busycnt) {
@@ -302,6 +360,17 @@
 	struct buf *nbp;
 	int error, sz;
 
+	/* DF-0859: abp is the fnode fn_ab (called from hpfs_addextent with
+	 * rabp = &hp->h_fn.fn_ab); validate ab_busycnt against the fnode
+	 * (96-byte data area) container max before the bcopy uses it.
+	 * The destination alsec container is larger, so the fnode max is the
+	 * binding constraint. */
+	if (hpfs_ab_busycnt_ok(abp, /*is_fnode=*/1) != 0) {
+		kprintf("hpfs_alblk2alsec: forged ab_busycnt %u > fnode max\n",
+		    abp->ab_busycnt);
+		return (EINVAL);
+	}
+
 	error = hpfs_allocalsec(hpmp, 0, &nbp);
 	if (error)
 		return (error);