Module Name:    src
Committed By:   martin
Date:           Sun Jun 24 09:32:03 UTC 2018

Modified Files:
        src/usr.sbin/installboot/arch [netbsd-8]: i386.c

Log Message:
Pull up following revision(s) (requested by kamil in ticket #898):

        usr.sbin/installboot/arch/i386.c: revision 1.41

Fix integer overflow in installboot(8)
Add a sanity check of the disk_buf first three bytes. The original code on
a disk with nul bytes was causing integer overflow and thus calling the
memcmp(3) functin in is_zero() with enormous length.
Verity that the 0th byte is JMP, 1th a signed byte >= 9 to prevent overflow
and 2th byte NOP.

Add a comment explaining the check.

Detected with MKSANITIZER and ASan.


To generate a diff of this commit:
cvs rdiff -u -r1.40 -r1.40.20.1 src/usr.sbin/installboot/arch/i386.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/usr.sbin/installboot/arch/i386.c
diff -u src/usr.sbin/installboot/arch/i386.c:1.40 src/usr.sbin/installboot/arch/i386.c:1.40.20.1
--- src/usr.sbin/installboot/arch/i386.c:1.40	Fri Jun 14 03:54:43 2013
+++ src/usr.sbin/installboot/arch/i386.c	Sun Jun 24 09:32:03 2018
@@ -1,4 +1,4 @@
-/* $NetBSD: i386.c,v 1.40 2013/06/14 03:54:43 msaitoh Exp $ */
+/* $NetBSD: i386.c,v 1.40.20.1 2018/06/24 09:32:03 martin Exp $ */
 
 /*-
  * Copyright (c) 2003 The NetBSD Foundation, Inc.
@@ -35,7 +35,7 @@
 
 #include <sys/cdefs.h>
 #if !defined(__lint)
-__RCSID("$NetBSD: i386.c,v 1.40 2013/06/14 03:54:43 msaitoh Exp $");
+__RCSID("$NetBSD: i386.c,v 1.40.20.1 2018/06/24 09:32:03 martin Exp $");
 #endif /* !__lint */
 
 #include <sys/param.h>
@@ -418,8 +418,19 @@ i386_setboot(ib_params *params)
 			return 0;
 		}
 
-		/* Find size of old BPB, and copy into new bootcode */
-		if (!is_zero(disk_buf.b + 3 + 8, disk_buf.b[1] - 1 - 8)) {
+		/*
+		 * Find size of old BPB, and copy into new bootcode
+		 *
+		 * The 2nd byte (b[1]) contains jmp short relative offset.
+		 * If it is zero or some invalid input that is smaller than 9,
+		 * it will cause overflow and call is_zero() with enormous size.
+		 * Add a paranoid check to prevent this scenario.
+		 *
+		 * Verify that b[0] contains JMP (0xeb) and b[2] NOP (0x90).
+		 */
+		if (disk_buf.b[0] == 0xeb && disk_buf.b[1] >= 9 &&
+		    disk_buf.b[2] == 0x90 &&
+		    !is_zero(disk_buf.b + 3 + 8, disk_buf.b[1] - 1 - 8)) {
 			struct mbr_bpbFAT16 *bpb = (void *)(disk_buf.b + 3 + 8);
 			/* Check enough space before the FAT for the bootcode */
 			u = le16toh(bpb->bpbBytesPerSec)

Reply via email to