[PASE] Validate initiatorRandom length in HandlePBKDFParamResponse#72741
Merged
mergify[bot] merged 1 commit intoJun 26, 2026
Merged
Conversation
HandlePBKDFParamResponse() reads the peer's initiatorRandom field into a fixed 32-byte buffer via TLVReader::GetBytes(random, sizeof(random)). GetBytes() copies only the element's actual length and leaves the rest of the buffer unwritten, so an initiatorRandom shorter than kPBKDFParamRandomNumberSize leaves random[len..32) uninitialized. That tail is then read by the following ByteSpan(random).data_equal(...) comparison, an uninitialized-stack read (flagged by MemorySanitizer). The two analogous reads already guard against this with an explicit length check first: the responderRandom field a few lines below, and the initiatorRandom field in HandlePBKDFParamRequest(). Only this read was missing the check. Add it so a malformed PBKDFParamResponse is rejected with CHIP_ERROR_INVALID_TLV_ELEMENT before the buffer is read, matching the surrounding code. No behavioral change for conformant peers, which always send a 32-byte initiatorRandom.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request updates PASESession.cpp to verify that the length of the initiator's random value in the TLV reader matches kPBKDFParamRandomNumberSize before retrieving the bytes. This prevents potential issues with invalid TLV element sizes. There are no review comments, so I have no feedback to provide.
|
PR #72741: Size comparison from 413ff63 to acd9ed7 Full report (33 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, psoc6, qpg, realtek, stm32, telink)
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #72741 +/- ##
==========================================
+ Coverage 56.76% 56.79% +0.03%
==========================================
Files 1634 1642 +8
Lines 112660 112758 +98
Branches 13144 13139 -5
==========================================
+ Hits 63946 64041 +95
- Misses 48714 48717 +3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
jmartinez-silabs
approved these changes
Jun 26, 2026
ForemanZack-CableLabs
approved these changes
Jun 26, 2026
greens
approved these changes
Jun 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
PASESession::HandlePBKDFParamResponse()reads the peer'sinitiatorRandomfield into a fixed-size 32-byte buffer:TLVReader::GetBytes(buf, bufSize)copies only the element's actual length and does not zero the remainder of the buffer. If the receivedinitiatorRandomelement is shorter thankPBKDFParamRandomNumberSize,random[len..32)is left uninitialized and is then read by thedata_equal()comparison — an uninitialized-stack read (surfaced by MemorySanitizer).The two analogous reads already validate the element length before reading:
responderRandomfield a few lines below in the same function, andinitiatorRandomfield inHandlePBKDFParamRequest().Only the
initiatorRandomread inHandlePBKDFParamResponse()was missing the check.Change
Add the same
GetLength() == kPBKDFParamRandomNumberSizeguard before reading the buffer, so a malformed message is rejected withCHIP_ERROR_INVALID_TLV_ELEMENTbefore the read, consistent with the surrounding code.Impact
Low / defense-in-depth. A conformant peer always sends a 32-byte
initiatorRandom, so the new check never affects a valid handshake; it only hardens the malformed-input path, which was already rejected — just after the uninitialized read instead of before it.Testing
No new test added: the change is a one-line guard identical to the
responderRandomlength check already present (and exercised by the existingTestPASESessionPASE handshake tests) a few lines below in the same function, and it does not change behavior for valid handshakes (a conformant peer always sends a 32-byteinitiatorRandom). Build and the existing PASE unit tests run in CI.