[edk2-devel] [PATCH v1 00/11] Test against invalid pointers in acpiview

Krzysztof Koch posted 11 patches 4 years, 7 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c              |  9 ++-
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c | 43 ++++++++++++++
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c | 14 +++++
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, 272 insertions(+), 7 deletions(-)
[edk2-devel] [PATCH v1 00/11] Test against invalid pointers in acpiview
Posted by Krzysztof Koch 4 years, 7 months ago
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_v1

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 | 14 +++++
 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, 272 insertions(+), 7 deletions(-)

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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#45723): https://edk2.groups.io/g/devel/message/45723
Mute This Topic: https://groups.io/mt/32886564/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1 00/11] Test against invalid pointers in acpiview
Posted by Liming Gao 4 years, 7 months ago
Krzysztof:
   Can you submit BZ in https://bugzilla.tianocore.org/ for this change? 

Thanks
Liming
>-----Original Message-----
>From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>Krzysztof Koch
>Sent: Thursday, August 15, 2019 9:11 PM
>To: devel@edk2.groups.io
>Cc: Carsey, Jaben <jaben.carsey@intel.com>; Ni, Ray <ray.ni@intel.com>; Gao,
>Zhichao <zhichao.gao@intel.com>; Sami.Mujawar@arm.com;
>Matteo.Carlini@arm.com; nd@arm.com
>Subject: [edk2-devel] [PATCH v1 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_
>v1
>
>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 |
>14 +++++
> 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, 272 insertions(+), 7 deletions(-)
>
>--
>'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
>
>
>
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#45782): https://edk2.groups.io/g/devel/message/45782
Mute This Topic: https://groups.io/mt/32886564/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1 00/11] Test against invalid pointers in acpiview
Posted by Krzysztof Koch 4 years, 7 months ago
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2089

Hi Liming,

Sure, no problem.

Kind regards,

Krzysztof

-----Original Message-----
From: Gao, Liming <liming.gao@intel.com> 
Sent: Friday, August 16, 2019 5:03
To: devel@edk2.groups.io; Krzysztof Koch <Krzysztof.Koch@arm.com>
Cc: Carsey, Jaben <jaben.carsey@intel.com>; Ni, Ray <ray.ni@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>; Sami Mujawar <Sami.Mujawar@arm.com>; Matteo Carlini <Matteo.Carlini@arm.com>; nd <nd@arm.com>
Subject: RE: [edk2-devel] [PATCH v1 00/11] Test against invalid pointers in acpiview

Krzysztof:
   Can you submit BZ in https://bugzilla.tianocore.org/ for this change? 

Thanks
Liming
>-----Original Message-----
>From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of 
>Krzysztof Koch
>Sent: Thursday, August 15, 2019 9:11 PM
>To: devel@edk2.groups.io
>Cc: Carsey, Jaben <jaben.carsey@intel.com>; Ni, Ray <ray.ni@intel.com>; 
>Gao, Zhichao <zhichao.gao@intel.com>; Sami.Mujawar@arm.com; 
>Matteo.Carlini@arm.com; nd@arm.com
>Subject: [edk2-devel] [PATCH v1 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_
>v1
>
>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 
>|
>14 +++++
> 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, 272 insertions(+), 7 deletions(-)
>
>--
>'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
>
>
>
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#45797): https://edk2.groups.io/g/devel/message/45797
Mute This Topic: https://groups.io/mt/32886564/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1 00/11] Test against invalid pointers in acpiview
Posted by Gao, Zhichao 4 years, 7 months ago
For 1-2, 4, 6-11: Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>

Thanks,
Zhichao

> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Krzysztof Koch
> Sent: Thursday, August 15, 2019 9:11 PM
> To: devel@edk2.groups.io
> Cc: Carsey, Jaben <jaben.carsey@intel.com>; Ni, Ray <ray.ni@intel.com>;
> Gao, Zhichao <zhichao.gao@intel.com>; Sami.Mujawar@arm.com;
> Matteo.Carlini@arm.com; nd@arm.com
> Subject: [edk2-devel] [PATCH v1 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_
> v1
> 
> 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
> | 14 +++++
> 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, 272 insertions(+), 7 deletions(-)
> 
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> 
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#46130): https://edk2.groups.io/g/devel/message/46130
Mute This Topic: https://groups.io/mt/32886564/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1 00/11] Test against invalid pointers in acpiview
Posted by Sami Mujawar 4 years, 7 months ago
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar

-----Original Message-----
From: Krzysztof Koch <krzysztof.koch@arm.com> 
Sent: 15 August 2019 02:11 PM
To: devel@edk2.groups.io
Cc: jaben.carsey@intel.com; ray.ni@intel.com; zhichao.gao@intel.com; Sami Mujawar <Sami.Mujawar@arm.com>; Matteo Carlini <Matteo.Carlini@arm.com>; nd <nd@arm.com>
Subject: [PATCH v1 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_v1

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 | 14 +++++  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, 272 insertions(+), 7 deletions(-)

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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#46031): https://edk2.groups.io/g/devel/message/46031
Mute This Topic: https://groups.io/mt/32886564/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-