Modify the PPTT table parsing logic to prevent reading past the ACPI buffer lengths provided.
Check if the Number of Private Resources specified in the Processor Hierarchy Node (Type 0) is possible given the Type 0 Structure's buffer length. Make sure that the processor topology structure's buffer fits in the PPTT table buffer before its contents are dumped. Prevent buffer overruns when reading the processor topology structure's header. References: - ACPI 6.3, January 2019, Section 5.2.29 Signed-off-by: Krzysztof Koch <krzysztof.k...@arm.com> --- Notes: v1: - Prevent buffer overruns in PPTT acpiview parser [Krzysztof] ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c | 38 ++++++++++++++------ 1 file changed, 27 insertions(+), 11 deletions(-) diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c index cec57be55e77096f9448f637ea129af2b42111ad..6254b9913fffb429fc54bb1301bf3e4b2e5bf161 100644 --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c @@ -252,7 +252,6 @@ DumpProcessorHierarchyNodeStructure ( ) { UINT32 Offset; - UINT8* PrivateResourcePtr; UINT32 Index; CHAR16 Buffer[OUTPUT_FIELD_COLUMN_WIDTH]; @@ -265,8 +264,23 @@ DumpProcessorHierarchyNodeStructure ( PARSER_PARAMS (ProcessorHierarchyNodeStructureParser) ); - PrivateResourcePtr = Ptr + Offset; + // Make sure the Private Resource array lies inside this structure + if (Offset + (*NumberOfPrivateResources * sizeof (UINT32)) > Length) { + IncrementErrorCount (); + Print ( + L"ERROR: Invalid Number of Private Resources. " \ + L"PrivateResourceCount = %d. RemainingBufferLength = %d. " \ + L"Parsing of this structure aborted.\n", + *NumberOfPrivateResources, + Length - Offset + ); + return; + } + Index = 0; + + // Parse the specified number of private resource references or the Processor + // Hierarchy Node length. Whichever is minimum. while (Index < *NumberOfPrivateResources) { UnicodeSPrint ( Buffer, @@ -278,10 +292,10 @@ DumpProcessorHierarchyNodeStructure ( PrintFieldName (4, Buffer); Print ( L"0x%x\n", - *((UINT32*) PrivateResourcePtr) + *((UINT32*)(Ptr + Offset)) ); - PrivateResourcePtr += sizeof(UINT32); + Offset += sizeof (UINT32); Index++; } } @@ -382,19 +396,21 @@ ParseAcpiPptt ( 0, NULL, ProcessorTopologyStructurePtr, - 4, // Length of the processor topology structure header is 4 bytes + AcpiTableLength - Offset, PARSER_PARAMS (ProcessorTopologyStructureHeaderParser) ); - if ((Offset + (*ProcessorTopologyStructureLength)) > AcpiTableLength) { + // Make sure the PPTT structure lies inside the table + if ((Offset + *ProcessorTopologyStructureLength) > AcpiTableLength) { IncrementErrorCount (); Print ( - L"ERROR: Invalid processor topology structure length:" - L" Type = %d, Length = %d\n", - *ProcessorTopologyStructureType, - *ProcessorTopologyStructureLength + L"ERROR: Invalid PPTT structure length. " \ + L"ProcessorTopologyStructureLength = %d. " \ + L"RemainingTableBufferLength = %d. PPTT parsing aborted.\n", + *ProcessorTopologyStructureLength, + AcpiTableLength - Offset ); - break; + return; } PrintFieldName (2, L"* Structure Offset *"); -- 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)' -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#44759): https://edk2.groups.io/g/devel/message/44759 Mute This Topic: https://groups.io/mt/32676847/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-