[src/trunk]: src/usr.sbin/installboot/arch Fix integer overflow in installboo...

[kamil <kamil%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/803ef2d6f932
branches:  trunk
changeset: 323630:803ef2d6f932
user:      kamil <kamil%NetBSD.org@localhost>
date:      Sat Jun 23 14:15:57 2018 +0000

description:
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.

diffstat:

 usr.sbin/installboot/arch/i386.c |  19 +++++++++++++++----
 1 files changed, 15 insertions(+), 4 deletions(-)

diffs (40 lines):

diff -r 5c309288d62a -r 803ef2d6f932 usr.sbin/installboot/arch/i386.c
--- a/usr.sbin/installboot/arch/i386.c  Sat Jun 23 14:14:42 2018 +0000
+++ b/usr.sbin/installboot/arch/i386.c  Sat Jun 23 14:15:57 2018 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: i386.c,v 1.40 2013/06/14 03:54:43 msaitoh Exp $ */
+/* $NetBSD: i386.c,v 1.41 2018/06/23 14:15:57 kamil 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.41 2018/06/23 14:15:57 kamil Exp $");
 #endif /* !__lint */
 
 #include <sys/param.h>
@@ -418,8 +418,19 @@
                        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)