edk2 icon indicating copy to clipboard operation
edk2 copied to clipboard

Published 20 hours ago • verified publisher icon tianocore

Add Signature check and fix buffer overrun issue in FtwGetLastWriteHeader

Open sureshkumarpMSFT opened this issue 1 year ago • 2 comments

Description

Add Signature check and fix buffer overrun issue in FtwGetLastWriteHeader

<Include a description of the change and why this change was made.>

  • This PR aims to prevent a buffer overrun issue found in FtwGetLastWriteHeader function. As per the current code, when there is a malformed blocks (with all bytes as 0s) then code Offset += FTW_WRITE_TOTAL_SIZE (FtwHeader->NumberOfWrites, FtwHeader->PrivateDataSize) would access beyond FtwWorkSpaceSize.

  • Also added the signature check to validate work space

<For each item, place an "x" in between [ and ] if true. Example: [x] (you can also check items in GitHub UI)>

<Create the PR as a Draft PR if it is only created to run CI checks.>

<Delete lines in <> tags before creating the PR.>

  • [ ] Breaking change?
    • Breaking change - Does this PR cause a break in build or boot behavior?
    • Examples: Does it add a new library class or move a module to a different repo.
  • [x ] Impacts security?
    • Security - Does this PR have a direct security impact?
    • Harden the access within FtwWorkSpace
  • [ ] Includes tests?
    • Tests - Does this PR include any explicit test code?
    • Examples: Unit tests or integration tests.

How This Was Tested

Tested this in platform which has a SPI chip with improper firmware blocks. Without this fix boot crashed and with this change system continued to boot.

Integration Instructions

<Describe how these changes should be integrated. Use N/A if nothing is required.>

sureshkumarpMSFT avatar Aug 05 '24 15:08 sureshkumarpMSFT

The checker is good. But, the commit message doesn't follow the style. Please refer to https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format

lgao4 avatar Aug 06 '24 01:08 lgao4

Thanks @lgao4

I have updated the commit message. Can you please check again.

sureshkumarpMSFT avatar Aug 06 '24 16:08 sureshkumarpMSFT

@mergify refresh

mdkinney avatar Aug 29 '24 16:08 mdkinney

refresh

✅ Pull request refreshed

mergify[bot] avatar Aug 29 '24 16:08 mergify[bot]

@mergify rebase

mdkinney avatar Aug 30 '24 04:08 mdkinney

rebase

✅ Branch has been successfully rebased

mergify[bot] avatar Aug 30 '24 04:08 mergify[bot]

@mergify refresh

mdkinney avatar Aug 30 '24 19:08 mdkinney

refresh

✅ Pull request refreshed

mergify[bot] avatar Aug 30 '24 19:08 mergify[bot]

I have asked Mergify support to evaluate why this PR is not being merged.

mdkinney avatar Sep 02 '24 16:09 mdkinney

@mergifyio requeue

mdkinney avatar Sep 03 '24 14:09 mdkinney

requeue

✅ The queue state of this pull request has been cleaned. It can be re-embarked automatically

mergify[bot] avatar Sep 03 '24 14:09 mergify[bot]