[fuzz] Guard PASE fuzz harness against uninitialized verifier on Generate() failure#72749
[fuzz] Guard PASE fuzz harness against uninitialized verifier on Generate() failure#72749Alami-Amine wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request improves MemorySanitizer (MSan) support and prevents false positives during fuzz testing. Specifically, it automatically enables CHIP_MEMORY_SANITIZER_ENABLED when the compiler detects MSan (such as in OSS-Fuzz builds). Additionally, in FuzzPASE_PW.cpp, it replaces ignored return values of Generate() with VerifyOrReturn checks to bail early if the generation fails, preventing MSan false positives caused by uninitialized verifier reads. There are no review comments, and I have no feedback to provide.
…rate() failure FuzzPASE_PW.cpp's Pake1/Pake2/Pake3 harnesses ignored mPASEVerifier.Generate()'s result and then read the verifier via BeginVerifier()/HandleMsg*(). Generate() can fail because the fuzz domains intentionally include out-of-range PBKDF iteration counts and salt lengths; on failure mPASEVerifier stays uninitialized and is read downstream (an MSan use-of-uninitialized-value flagged by the OSS-Fuzz memory build). Guard each site with VerifyOrReturn; production always checks Generate(), so this is a test-harness gap, not a product bug.
f2f94f7 to
d27ba35
Compare
|
PR #72749: Size comparison from fc4a9e8 to d27ba35 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 #72749 +/- ##
==========================================
- Coverage 56.79% 56.61% -0.19%
==========================================
Files 1642 1642
Lines 112757 113137 +380
Branches 13139 13243 +104
==========================================
+ Hits 64041 64048 +7
- Misses 48716 49089 +373 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Address review: ReturnOnFailure takes the CHIP_ERROR directly instead of VerifyOrReturn(... == CHIP_NO_ERROR), at all three Generate() sites.
|
PR #72749: Size comparison from fc4a9e8 to 0e01ae3 Full report (33 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, psoc6, qpg, realtek, stm32, telink)
|
Summary
The PASE fuzz harness reads an uninitialized verifier when
Generate()fails — flagged by MSan in the OSS-Fuzzmemorybuild (fuzz-PASE-pw@…Pake2/Pake3).FuzzPASE_PW.cpp'sFuzzHandlePake1/2/3ignored the result ofpairingAccessory.mPASEVerifier.Generate()and then read the verifier viaBeginVerifier()/HandleMsg*().Generate()can fail because the Pake fuzz domains intentionally include out-of-range PBKDF iteration counts and salt lengths; on failuremPASEVerifierstays uninitialized and is read downstream.Guard each site with
VerifyOrReturn(... == CHIP_NO_ERROR)so the harness bails when the precondition can't be established. Production always checksGenerate(), so this is a test-harness gap, not a product bug.(CI flagged Pake2/Pake3; Pake1 has the identical pattern and is fixed for consistency.)
Testing
memory(MemorySanitizer)check_buildrun, which reporteduse-of-uninitialized-valuereachingbin2bnviaSpake2p::FELoad/BeginVerifierfrom these harnesses, with the origin in the harnesspairingAccessoryobject.Generate()fails — i.e. for the out-of-range PBKDF iteration/salt values in the fuzz domains, where the accessory verifier precondition could not be established anyway. In-range inputs (the bulk of the domain) exercise the message handlers exactly as before.memorybuild re-run with this change applied (the targets that previously aborted now run cleanly).