From f06734e55df48d896438e08cc283e3806bcca521 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Herm=C3=A8s=20B=C3=A9lusca-Ma=C3=AFto?=
 <hermes.belusca-maito@reactos.org>
Date: Tue, 9 May 2017 15:31:53 +0000
Subject: [PATCH] [USETUP]: Improve the bootsector validity check performed in
 IsThereAValidBootSector: - Check for the first 3 bytes (and not 4) of the
 bootsector to not be zero (that's our criterium for a "valid instruction").
 Therefore, a bootsector starting with "00 00 00 xx" (with xx the first byte
 of a volume identifier) is detected as invalid (because the BIOS won't be
 able to run it anyways) and therefore, needs to be overwritten. - Check that
 its last 2 bytes are the valid 0xAA55 signature. These improvements were
 suggested by Serge Gautherie and Peter Hater. CORE-4870 CORE-12672 CORE-13188
 - Move a DPRINT1 around.

svn path=/trunk/; revision=74512
---
 reactos/base/setup/usetup/bootsup.c          | 35 +++++++++++---------
 reactos/base/setup/usetup/interface/usetup.c |  5 ++-
 2 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/reactos/base/setup/usetup/bootsup.c b/reactos/base/setup/usetup/bootsup.c
index 8ed151c6a79..7b4629bb5ad 100644
--- a/reactos/base/setup/usetup/bootsup.c
+++ b/reactos/base/setup/usetup/bootsup.c
@@ -630,12 +630,14 @@ BOOLEAN
 IsThereAValidBootSector(PWSTR RootPath)
 {
     /*
-     * Check the first DWORD (4 bytes) of the bootsector for a potential
-     * "valid" instruction (the BIOS starts execution of the bootsector
-     * at its beginning). Currently the criterium is that this DWORD must
-     * be non-zero.
+     * We first demand that the bootsector has a valid signature at its end.
+     * We then check the first 3 bytes (as a ULONG) of the bootsector for a
+     * potential "valid" instruction (the BIOS starts execution of the bootsector
+     * at its beginning). Currently this criterium is that this ULONG must be
+     * non-zero. If both these tests pass, then the bootsector is valid; otherwise
+     * it is invalid and certainly needs to be overwritten.
      */
-
+    BOOLEAN IsValid = FALSE;
     NTSTATUS Status;
     UNICODE_STRING Name;
     OBJECT_ATTRIBUTES ObjectAttributes;
@@ -666,10 +668,9 @@ IsThereAValidBootSector(PWSTR RootPath)
                         0,
                         FILE_SYNCHRONOUS_IO_NONALERT);
     if (!NT_SUCCESS(Status))
-    {
-        RtlFreeHeap(ProcessHeap, 0, BootSector);
-        return FALSE; // Status;
-    }
+        goto Quit;
+
+    RtlZeroMemory(BootSector, SECTORSIZE);
 
     FileOffset.QuadPart = 0ULL;
     Status = NtReadFile(FileHandle,
@@ -682,16 +683,20 @@ IsThereAValidBootSector(PWSTR RootPath)
                         &FileOffset,
                         NULL);
     NtClose(FileHandle);
+    if (!NT_SUCCESS(Status))
+        goto Quit;
 
-    Instruction = *(PULONG)BootSector;
+    /* Check the instruction; we use a ULONG to read three bytes */
+    Instruction = (*(PULONG)BootSector) & 0x00FFFFFF;
+    IsValid = (Instruction != 0x00000000);
 
+    /* Check the bootsector signature */
+    IsValid &= (*(PUSHORT)(BootSector + 0x1fe) == 0xaa55);
+
+Quit:
     /* Free the boot sector */
     RtlFreeHeap(ProcessHeap, 0, BootSector);
-
-    if (!NT_SUCCESS(Status))
-        return FALSE; // Status;
-
-    return (Instruction != 0x00000000);
+    return IsValid; // Status;
 }
 
 NTSTATUS
diff --git a/reactos/base/setup/usetup/interface/usetup.c b/reactos/base/setup/usetup/interface/usetup.c
index f9df8fd0e1b..03efba566cb 100644
--- a/reactos/base/setup/usetup/interface/usetup.c
+++ b/reactos/base/setup/usetup/interface/usetup.c
@@ -4442,9 +4442,6 @@ BootLoaderHarddiskMbrPage(PINPUT_RECORD Ir)
     wcscpy(SourceMbrPathBuffer, SourceRootPath.Buffer);
     wcscat(SourceMbrPathBuffer, L"\\loader\\dosmbr.bin");
 
-    DPRINT1("Install MBR bootcode: %S ==> %S\n",
-            SourceMbrPathBuffer, DestinationDevicePathBuffer);
-
     if (IsThereAValidBootSector(DestinationDevicePathBuffer))
     {
         /* Save current MBR */
@@ -4460,6 +4457,8 @@ BootLoaderHarddiskMbrPage(PINPUT_RECORD Ir)
         }
     }
 
+    DPRINT1("Install MBR bootcode: %S ==> %S\n",
+            SourceMbrPathBuffer, DestinationDevicePathBuffer);
     Status = InstallMbrBootCodeToDisk(SourceMbrPathBuffer,
                                       DestinationDevicePathBuffer);
     if (!NT_SUCCESS(Status))