-
Notifications
You must be signed in to change notification settings - Fork 2.2k
[NXP][platform][common] add WPA3 support #39411
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
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Martin Girardot <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request aims to add WPA3-SAE support for NXP platforms by updating how Wi-Fi network credentials are provisioned. The core change involves populating a new password
field in the network security structure, intended for WPA3's potentially longer passwords, alongside the existing psk
field.
While adding WPA3 support is a valuable enhancement, the current implementation has critical memory safety concerns. Specifically, there are potential buffer overflows when copying the key to both the psk
and the new password
fields if the provided keyLen
exceeds their respective buffer capacities. These issues need to be addressed before merging.
Summary of Findings
- Potential Buffer Overflow in
psk
field copy: Thememcpy
topNetworkData->security.psk
(line 645) can cause a buffer overflow ifkeyLen
(now potentially for a longer WPA3 password) exceeds thepsk
buffer's capacity. This is a critical memory safety issue. - Potential Buffer Overflow in new
password
field copy: The newly addedmemcpy
topNetworkData->security.password
(line 648) can also cause a buffer overflow ifkeyLen
exceeds thepassword
buffer's capacity. This also needs to be safeguarded to prevent memory corruption.
Merge Readiness
This pull request introduces important WPA3 support. However, there are critical buffer overflow vulnerabilities identified in the way network keys are copied. These issues must be addressed to ensure memory safety before the PR can be considered for merging. As an AI reviewer, I am not authorized to approve pull requests; please ensure these changes are reviewed and approved by other maintainers after the identified issues are resolved.
PR #39411: Size comparison from 0a6c200 to 4c8586c Full report (14 builds for cc13x4_26x4, cc32xx, nrfconnect, qpg, stm32, tizen)
|
06d7494
to
93c0bfc
Compare
PR #39411: Size comparison from d74efe3 to 8ce4ae9 Increases above 0.2%:
Full report (70 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, stm32, telink, tizen)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Do not memcpy into a buffer from outside data without bound-checking against the size of the buffer receiving the data.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Signed-off-by: Martin Girardot <[email protected]>
Description
Add WPA3-SAE support for NXP platforms
Testing
Wifi commissioning using WPA3-SAE security AP, rw61x NXP platform