DragonFlyBSD Kernel Audit
DF-0861 / fix.diff
← back to finding ↓ download raw
diff --git a/sys/vfs/hpfs/hpfs.h b/sys/vfs/hpfs/hpfs.h
--- a/sys/vfs/hpfs/hpfs.h
+++ b/sys/vfs/hpfs/hpfs.h
@@ -307,6 +307,10 @@
 	struct cpiblk	s_cpi[0x1F];	/* Array of CPI blocks */
 };
 
+/* Sanity bounds for untrusted on-disk code-page counts (DF-0861). */
+#define	CPIS_NCPI		0x1F		/* max cpiblk per cpisec (s_cpi[]) */
+#define	HPFS_SP_CPINUM_MAX	256		/* sane upper bound for sp_cpinum */
+
 struct hpfsmount {
 	struct sublock	hpm_su;
 	struct spblock	hpm_sp;
diff --git a/sys/vfs/hpfs/hpfs_subr.c b/sys/vfs/hpfs/hpfs_subr.c
--- a/sys/vfs/hpfs/hpfs_subr.c
+++ b/sys/vfs/hpfs/hpfs_subr.c
@@ -271,7 +271,20 @@
 		}
 	}
 
+	/*
+	 * Validate the on-disk code-page counts before using them as
+	 * allocation and loop bounds.  sp_cpinum is an untrusted u32; a
+	 * forged value would either make the kmalloc wrap/huge or -- worse
+	 * -- let the inner loop below write past the cpicnt-sized hpm_cpdblk
+	 * array (that loop is bounded by cpisp->s_cpicnt, a SEPARATE
+	 * untrusted u32).  Cap sp_cpinum to reject forged superblocks.
+	 */
 	cpicnt = hpmp->hpm_sp.sp_cpinum;
+	if (cpicnt > HPFS_SP_CPINUM_MAX) {
+		kprintf("hpfs_cpinit: forged sp_cpinum %d > max %d\n",
+			cpicnt, HPFS_SP_CPINUM_MAX);
+		return (EINVAL);
+	}
 
 	hpmp->hpm_cpdblk = kmalloc(cpicnt * sizeof(struct cpdblk), M_HPFSMNT,
 				   M_WAITOK);
@@ -289,7 +302,9 @@
 		cpisp = (struct cpisec *)bp->b_data;
 
 		cpibp = cpisp->s_cpi;
-		for (i=0; i<cpisp->s_cpicnt; i++, cpicnt --, cpdbp++, cpibp++) {
+		if (cpisp->s_cpicnt > CPIS_NCPI)
+			cpisp->s_cpicnt = CPIS_NCPI;
+		for (i=0; i<cpisp->s_cpicnt && cpicnt > 0; i++, cpicnt --, cpdbp++, cpibp++) {
 			dprintf(("hpfs_cpinit: Country: %d, CP: %d (%d)\n",
 				 cpibp->b_country, cpibp->b_cpid, 
 				 cpibp->b_vcpid));