ShellPkg: acpiview: Prevent infinite loop if structure length is 0

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2534

Extend validation of ACPI structure lengths which are read from the
ACPI table being parsed. Additionally check if the structure 'Length'
field value is positive. If not, stop parsing the faulting table.

Some ACPI tables define internal structures of variable size. The
'Length' field inside the substructure is used to update a pointer used
for table traversal. If the byte-length of the structure is equal to 0,
acpiview can enter an infinite loop. This condition can occur if, for
example, the zero-allocated ACPI table buffer is not fully populated.
This is typically a bug on the ACPI table writer side.

In short, this method helps acpiview recover gracefully from a
zero-valued ACPI structure length.

Signed-off-by: Krzysztof Koch <krzysztof.koch@arm.com>
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>

6 files changed, 47 insertions, 52 deletions

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c
index 0f730a306a..9df111ecaa 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c
@@ -1,7 +1,7 @@
 /** @file

   DBG2 table parser

 

-  Copyright (c) 2016 - 2019, ARM Limited. All rights reserved.

+  Copyright (c) 2016 - 2020, ARM Limited. All rights reserved.

   SPDX-License-Identifier: BSD-2-Clause-Patent

 

   @par Reference(s):

@@ -282,15 +282,16 @@ ParseAcpiDbg2 (
       return;

     }

 

-    // Make sure the Debug Device Information structure lies inside the table.

-    if ((Offset + *DbgDevInfoLen) > AcpiTableLength) {

+    // Validate Debug Device Information Structure length

+    if ((*DbgDevInfoLen == 0) ||

+        ((Offset + (*DbgDevInfoLen)) > AcpiTableLength)) {

       IncrementErrorCount ();

       Print (

-        L"ERROR: Invalid Debug Device Information structure length. " \

-          L"DbgDevInfoLen = %d. RemainingTableBufferLength = %d. " \

-          L"DBG2 parsing aborted.\n",

+        L"ERROR: Invalid Debug Device Information Structure length. " \

+          L"Length = %d. Offset = %d. AcpiTableLength = %d.\n",

         *DbgDevInfoLen,

-        AcpiTableLength - Offset

+        Offset,

+        AcpiTableLength

         );

       return;

     }

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
index 699a55b549..bdd30ff45c 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
@@ -1,7 +1,7 @@
 /** @file

   GTDT table parser

 

-  Copyright (c) 2016 - 2019, ARM Limited. All rights reserved.

+  Copyright (c) 2016 - 2020, ARM Limited. All rights reserved.

   SPDX-License-Identifier: BSD-2-Clause-Patent

 

   @par Reference(s):

@@ -327,15 +327,16 @@ ParseAcpiGtdt (
       return;

     }

 

-    // Make sure the Platform Timer is inside the table.

-    if ((Offset + *PlatformTimerLength) > AcpiTableLength) {

+    // Validate Platform Timer Structure length

+    if ((*PlatformTimerLength == 0) ||

+        ((Offset + (*PlatformTimerLength)) > AcpiTableLength)) {

       IncrementErrorCount ();

       Print (

         L"ERROR: Invalid Platform Timer Structure length. " \

-          L"PlatformTimerLength = %d. RemainingTableBufferLength = %d. " \

-          L"GTDT parsing aborted.\n",

+          L"Length = %d. Offset = %d. AcpiTableLength = %d.\n",

         *PlatformTimerLength,

-        AcpiTableLength - Offset

+        Offset,

+        AcpiTableLength

         );

       return;

     }

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
index 9d5d937c7b..9a006a0144 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
@@ -1,7 +1,7 @@
 /** @file

   IORT table parser

 

-  Copyright (c) 2016 - 2019, ARM Limited. All rights reserved.

+  Copyright (c) 2016 - 2020, ARM Limited. All rights reserved.

   SPDX-License-Identifier: BSD-2-Clause-Patent

 

   @par Reference(s):

@@ -687,14 +687,16 @@ ParseAcpiIort (
       return;

     }

 

-    // Make sure the IORT Node is inside the table

-    if ((Offset + (*IortNodeLength)) > AcpiTableLength) {

+    // Validate IORT Node length

+    if ((*IortNodeLength == 0) ||

+        ((Offset + (*IortNodeLength)) > AcpiTableLength)) {

       IncrementErrorCount ();

       Print (

-        L"ERROR: Invalid IORT node length. IortNodeLength = %d. " \

-          L"RemainingTableBufferLength = %d. IORT parsing aborted.\n",

+        L"ERROR: Invalid IORT Node length. " \

+          L"Length = %d. Offset = %d. AcpiTableLength = %d.\n",

         *IortNodeLength,

-        AcpiTableLength - Offset

+        Offset,

+        AcpiTableLength

         );

       return;

     }

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
index 438905cb24..f85d2b3653 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
@@ -1,7 +1,7 @@
 /** @file

   MADT table parser

 

-  Copyright (c) 2016 - 2019, ARM Limited. All rights reserved.

+  Copyright (c) 2016 - 2020, ARM Limited. All rights reserved.

   SPDX-License-Identifier: BSD-2-Clause-Patent

 

   @par Reference(s):

@@ -273,28 +273,16 @@ ParseAcpiMadt (
       return;

     }

 

-    // Make sure forward progress is made.

-    if (*MadtInterruptControllerLength < 2) {

+    // Validate Interrupt Controller Structure length

+    if ((*MadtInterruptControllerLength == 0) ||

+        ((Offset + (*MadtInterruptControllerLength)) > AcpiTableLength)) {

       IncrementErrorCount ();

       Print (

-        L"ERROR: Structure length is too small: " \

-          L"MadtInterruptControllerLength = %d. " \

-          L"MadtInterruptControllerType = %d. MADT parsing aborted.\n",

+        L"ERROR: Invalid Interrupt Controller Structure length. " \

+          L"Length = %d. Offset = %d. AcpiTableLength = %d.\n",

         *MadtInterruptControllerLength,

-        *MadtInterruptControllerType

-        );

-      return;

-    }

-

-    // Make sure the MADT structure lies inside the table

-    if ((Offset + *MadtInterruptControllerLength) > AcpiTableLength) {

-      IncrementErrorCount ();

-      Print (

-        L"ERROR: Invalid MADT structure length. " \

-          L"MadtInterruptControllerLength = %d. " \

-          L"RemainingTableBufferLength = %d. MADT parsing aborted.\n",

-        *MadtInterruptControllerLength,

-        AcpiTableLength - Offset

+        Offset,

+        AcpiTableLength

         );

       return;

     }

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c
index 675ba75f02..0db272c16a 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c
@@ -1,7 +1,7 @@
 /** @file

   PPTT table parser

 

-  Copyright (c) 2019, ARM Limited. All rights reserved.

+  Copyright (c) 2019 - 2020, ARM Limited. All rights reserved.

   SPDX-License-Identifier: BSD-2-Clause-Patent

 

   @par Reference(s):

@@ -425,15 +425,16 @@ ParseAcpiPptt (
       return;

     }

 

-    // Make sure the PPTT structure lies inside the table

-    if ((Offset + *ProcessorTopologyStructureLength) > AcpiTableLength) {

+    // Validate Processor Topology Structure length

+    if ((*ProcessorTopologyStructureLength == 0) ||

+        ((Offset + (*ProcessorTopologyStructureLength)) > AcpiTableLength)) {

       IncrementErrorCount ();

       Print (

-        L"ERROR: Invalid PPTT structure length. " \

-          L"ProcessorTopologyStructureLength = %d. " \

-          L"RemainingTableBufferLength = %d. PPTT parsing aborted.\n",

+        L"ERROR: Invalid Processor Topology Structure length. " \

+          L"Length = %d. Offset = %d. AcpiTableLength = %d.\n",

         *ProcessorTopologyStructureLength,

-        AcpiTableLength - Offset

+        Offset,

+        AcpiTableLength

         );

       return;

     }

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c
index 3613900ae3..6f66be68cc 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c
@@ -1,7 +1,7 @@
 /** @file

   SRAT table parser

 

-  Copyright (c) 2016 - 2019, ARM Limited. All rights reserved.

+  Copyright (c) 2016 - 2020, ARM Limited. All rights reserved.

   SPDX-License-Identifier: BSD-2-Clause-Patent

 

   @par Reference(s):

@@ -412,14 +412,16 @@ ParseAcpiSrat (
       return;

     }

 

-    // Make sure the SRAT structure lies inside the table

-    if ((Offset + *SratRALength) > AcpiTableLength) {

+    // Validate Static Resource Allocation Structure length

+    if ((*SratRALength == 0) ||

+        ((Offset + (*SratRALength)) > AcpiTableLength)) {

       IncrementErrorCount ();

       Print (

-        L"ERROR: Invalid SRAT structure length. SratRALength = %d. " \

-          L"RemainingTableBufferLength = %d. SRAT parsing aborted.\n",

+        L"ERROR: Invalid Static Resource Allocation Structure length. " \

+          L"Length = %d. Offset = %d. AcpiTableLength = %d.\n",

         *SratRALength,

-        AcpiTableLength - Offset

+        Offset,

+        AcpiTableLength

         );

       return;

     }