[src/trunk]: src/distrib/utils/sysinst The 16bit "bootmenu valid" magic is sl...

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

details:   https://anonhg.NetBSD.org/src/rev/30bb4c3ddf46
branches:  trunk
changeset: 754001:30bb4c3ddf46
user:      martin <martin%NetBSD.org@localhost>
date:      Thu Apr 15 22:55:15 2010 +0000

description:
The 16bit "bootmenu valid" magic is slightly week, collisions have been
seen in the wild. So, before accepting arbitrary strings from there,
validate at least slightly and ignore if the entries are not properly
0 terminated or contain controll characters.

diffstat:

 distrib/utils/sysinst/mbr.c |  54 ++++++++++++++++++++++++++++++++++++++------
 1 files changed, 46 insertions(+), 8 deletions(-)

diffs (77 lines):

diff -r c630d0205110 -r 30bb4c3ddf46 distrib/utils/sysinst/mbr.c
--- a/distrib/utils/sysinst/mbr.c       Thu Apr 15 20:56:32 2010 +0000
+++ b/distrib/utils/sysinst/mbr.c       Thu Apr 15 22:55:15 2010 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: mbr.c,v 1.82 2010/01/02 21:16:46 dsl Exp $ */
+/*     $NetBSD: mbr.c,v 1.83 2010/04/15 22:55:15 martin Exp $ */
 
 /*
  * Copyright 1997 Piermont Information Systems Inc.
@@ -1394,6 +1394,45 @@
        return unknown;
 }
 
+#ifdef BOOTSEL
+static int
+validate_and_set_names(mbr_info_t *mbri, const struct mbr_bootsel *src,
+    uint32_t ext_base)
+{
+       size_t i, l;
+       const unsigned char *p;
+
+       /*
+        * The 16 bit magic used to detect wether mbr_bootsel is valid
+        * or not is pretty week - collisions have been seen in the wild;
+        * but maybe it is just foreign tools corruption reminiscents
+        * of NetBSD MBRs. Anyway, before accepting a boot menu definition,
+        * make sure it is kinda "sane".
+        */
+
+       for (i = 0; i < MBR_PART_COUNT; i++) {
+               /*
+                * Make sure the name does not contain controll chars
+                * (not using iscntrl due to minimalistic locale support
+                * in miniroot environments) and is properly 0-terminated.
+                */
+               for (l = 0, p = (const unsigned char *)&src->mbrbs_nametab[i];
+                   *p != 0; l++, p++) {
+                       if (l > MBR_BS_PARTNAMESIZE)
+                               return 0;
+                       if (*p < ' ')   /* hacky 'iscntrl' */
+                               return 0;
+               }
+       }
+
+       memcpy(&mbri->mbrb, src, sizeof(*src));
+
+       if (ext_base == 0)
+               return mbri->mbrb.mbrbs_defkey - SCAN_1;
+       return 0;
+}
+#endif
+
 int
 read_mbr(const char *disk, mbr_info_t *mbri)
 {
@@ -1451,15 +1490,14 @@
 #if BOOTSEL
                if (mbrs->mbr_bootsel_magic == htole16(MBR_MAGIC)) {
                        /* old bootsel, grab bootsel info */
-                       mbri->mbrb = *(struct mbr_bootsel *)
-                               ((uint8_t *)mbrs + MBR_BS_OLD_OFFSET);
-                       if (ext_base == 0)
-                               bootkey = mbri->mbrb.mbrbs_defkey - SCAN_1;
+                       bootkey = validate_and_set_names(mbri, 
+                               (struct mbr_bootsel *)
+                               ((uint8_t *)mbrs + MBR_BS_OLD_OFFSET),
+                               ext_base);
                } else if (mbrs->mbr_bootsel_magic == htole16(MBR_BS_MAGIC)) {
                        /* new location */
-                       mbri->mbrb = mbrs->mbr_bootsel;
-                       if (ext_base == 0)
-                               bootkey = mbri->mbrb.mbrbs_defkey - SCAN_1;
+                       bootkey = validate_and_set_names(mbri,
+                           &mbrs->mbr_bootsel, ext_base);
                }
                /* Save original flags for mbr code update tests */
                mbri->oflags = mbri->mbrb.mbrbs_flags;