Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integrating patch for bz4207 #10594

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

MadhavanJey
Copy link

Description

A malicious iSCSI target could reply to the iSCSI initiator with a malformed packet, causing out-of-bounds memory reads and writes. This most likely leads to a denial of service, as the write primitive should not be exploitable.

To Fix this,
In IScsiBuildKeyValueList, check if we have any data left (Len > 0) before advancing the Data pointer and reducing Len. Avoids wrapping Len. Also replace the AsciiStrLen() call with an open-coded loop which likewise checks Len to make sure we don't overrun the buffer.

  • 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.
  • Impacts security?
    • Security - Does this PR have a direct security impact?
    • Examples: Crypto algorithm change or buffer overflow fix.
  • Includes tests?
    • Tests - Does this PR include any explicit test code?
    • Examples: Unit tests or integration tests.

How This Was Tested

<Describe the test(s) that were run to verify the changes.>

Integration Instructions

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

@leiflindholm
Copy link
Member

This change does not have a commit message.
Please write one, following https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format.

@leiflindholm
Copy link
Member

This change does not have a commit message. Please write one, following https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format.

(The description in the PR description is pretty good, but it needs to be in the commit message as well, and the DCO is also missing.)

@jkmathews jkmathews self-requested a review January 10, 2025 15:31
@jkmathews
Copy link

jkmathews commented Jan 11, 2025

Just an FYI to all reviewers, this patch was previously shared on the EDK2 dev mailing list, but it was never merged.
Link: https://edk2.groups.io/g/devel/message/106280

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants