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.

Sign up for GitHub

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

Jump to bottom

NetworkPkg/Dhcp6Dxe: Fix sanitizer issues #6420

Merged
merged 1 commit into from
Nov 11, 2024
Merged

NetworkPkg/Dhcp6Dxe: Fix sanitizer issues #6420

merged 1 commit into from
Nov 11, 2024

Conversation

mdkinney
Copy link
Member

@mdkinney mdkinney commented Nov 8, 2024

Description

  • EFI_DHCP6_DUID structure declares Duid[1], so the size of that structure is not large enough to hold an entire Duid. Instead, compute the correct size to allocate an EFI_DHCP6_DUID structure.
  • Dhcp6AppendOption() takes a length parameter that in network order. Update test cases to make sure a network order length is passed in. A value of 0x0004 was being passed in and was then converted to 0x0400 length and buffer overflow was detected.

Detected with use of address sanitizer in host based unit tests in draft PR #6408

  • 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

Run host based unit tests with address sanitizer enabled and the issue is resolved with this change

Integration Instructions

@github-actions github-actions bot added the impact:security This change has a direct security impact such as changing a crypto algorithm. label Nov 8, 2024
@mdkinney mdkinney requested a review from lgao4 November 8, 2024 19:05
@lgao4 lgao4 added the push Auto push patch series in PR if all checks pass label Nov 11, 2024
@mdkinney @lgao4
NetworkPkg/Dhcp6Dxe: Fix sanitizer issues
77b4e48 
* EFI_DHCP6_DUID structure declares Duid[1], so the size
  of that structure is not large enough to hold an entire
  Duid. Instead, compute the correct size to allocate an
  EFI_DHCP6_DUID structure.
* Dhcp6AppendOption() takes a length parameter that in
  network order. Update test cases to make sure a network
  order length is passed in. A value of 0x0004 was being
  passed in and was then converted to 0x0400 length and
  buffer overflow was detected.

Signed-off-by: Michael D Kinney <[email protected]>
@lgao4 lgao4 force-pushed the NetworkPkg_Dhcp6Dxe_UnitTest_FixSanitizerIssues branch from 8c743ed to 77b4e48 Compare November 11, 2024 04:54
@mergify mergify bot merged commit 599c830 into tianocore:master Nov 11, 2024
126 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lgao4 lgao4 lgao4 approved these changes

@ajfish ajfish Awaiting requested review from ajfish

@leiflindholm leiflindholm Awaiting requested review from leiflindholm

@SaloniKasbekar SaloniKasbekar Awaiting requested review from SaloniKasbekar

@Zclarkwilliams Zclarkwilliams Awaiting requested review from Zclarkwilliams

Assignees
No one assigned
Labels
impact:security This change has a direct security impact such as changing a crypto algorithm. push Auto push patch series in PR if all checks pass
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mdkinney @lgao4