Re: [PATCH v3 00/11] Test against invalid pointers in acpiview


Gao, Zhichao
 

Sorry for the misunderstanding before. The patch set is good to me.
Series: Reviewed-by: Zhichao Gao <zhichao.gao@...>

Thanks,
Zhichao

-----Original Message-----
From: Krzysztof Koch [mailto:krzysztof.koch@...]
Sent: Monday, January 20, 2020 7:14 PM
To: devel@edk2.groups.io
Cc: Ni, Ray <ray.ni@...>; Gao, Zhichao <zhichao.gao@...>;
Sami.Mujawar@...; Matteo.Carlini@...; nd@...
Subject: [PATCH v3 00/11] Test against invalid pointers in acpiview

Prevent the use of invalid pointers when parsing ACPI tables in the UEFI shell
acpiview tool.

The parsing of ACPI tables is often controlled with the values read earlier from
the same table. For example, the 'Offset' or 'Count' fields found in a structure
are later used to parse the substructures. If such fields lie outside the structure's
buffer length provided, then there is a possibility for a wild or dangling pointer.

Currently, if the ParseAcpi() function terminates early because the end of the
input table data buffer has been reached, then the pointers which were
supposed to be updated by this function are left untouched.
This is a security issue as the values pointed to by these pointers are later used
for flow control.

This patch series aims to solve this security issue by explicitly initializing any
pointers lying outside the input ACPI data buffer to NULL and testing for NULL
whenever these pointers are dereferenced.

Changes can be seet at:
https://github.com/KrzysztofKoch1/edk2/tree/612_add_pointer_validation_v3

Notes:
v3:
- Rebase on latest master [Krzysztof]

v2:
- Do not require FadtMinorRevision and X_DsdtAddress pointers to be
valid in FADT table parser [Zhichao]

v1:
- Validate static pointers in acpiview parsers before use [Krzysztof]

Krzysztof Koch (11):
ShellPkg: acpiview: Set ItemPtr to NULL for unprocessed table fields
ShellPkg: acpiview: RSDP: Validate global pointer before use
ShellPkg: acpiview: FADT: Validate global pointer before use
ShellPkg: acpiview: SLIT: Validate global pointer before use
ShellPkg: acpiview: SLIT: Validate System Locality count
ShellPkg: acpiview: SRAT: Validate global pointers before use
ShellPkg: acpiview: MADT: Validate global pointers before use
ShellPkg: acpiview: PPTT: Validate global pointers before use
ShellPkg: acpiview: IORT: Validate global pointers before use
ShellPkg: acpiview: GTDT: Validate global pointers before use
ShellPkg: acpiview: DBG2: Validate global pointers before use

ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c | 9 ++-
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c |
43 ++++++++++++++
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c | 21
+++----
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c | 37
++++++++++++
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c | 52
+++++++++++++++++
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c |
13 +++++
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c | 25
++++++++
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Rsdp/RsdpParser.c |
12 ++++
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c | 61
++++++++++++++++++--
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c | 13
+++++
10 files changed, 269 insertions(+), 17 deletions(-)

--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'

Join devel@edk2.groups.io to automatically receive all group messages.