Hello Crypto++ maintainers and community, I'm scc, and I've discovered an UndefinedBehaviorSanitizer (UBSan) violation in the ESIGN implementation while working on Issue #1338 ( https://github.com/weidai11/cryptopp/issues/1338). I've developed a comprehensive fix and validation infrastructure that I'd like to contribute.
*Issue Summary* - Primary Issue: UBSan detects null pointer passed to memcpy in esign.cpp:118 when an empty seed (size=0) is provided to InvertibleESIGNFunction::GenerateRandom. - Secondary Issue: This fix also resolves Issue #1338 (clang22 fortify warning) by using .data() and an explicit bounds assertion. *Environment* - Crypto++ Version: Current master (commit 60f81a77) - Operating System: Ubuntu (GitHub Actions: ubuntu-latest) - Compiler: Clang (default) and Clang 22 - Build Flags: -fsanitize=address,undefined for sanitizer testing; -O3 -D_FORTIFY_SOURCE=2 for fortify validation *Exact Error Message* esign.cpp:118:25: runtime error: null pointer passed as argument 2, which is declared to never be null /usr/include/string.h:44:28: note: nonnull attribute specified here SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior esign.cpp:118:25 *Stack Trace* #0 in CryptoPP::InvertibleESIGNFunction::GenerateRandom(...) /home/runner/work/cryptopp/cryptopp/esign.cpp:118:3 #1 in run_one(unsigned long, unsigned int) /home/runner/work/cryptopp/cryptopp/tests/esign_memcpy_asan.cpp:30:9 #2 in main /home/runner/work/cryptopp/cryptopp/tests/esign_memcpy_asan.cpp:47:17 *Root Cause* When seedParam.size() == 0, seedParam.begin() may return a null pointer, which violates the C++ standard requirement that memcpy must receive non-null pointers even when the copy size is 0. *Minimal Reproduction* A focused test exercising this path is available here: https://github.com/scc-tw/cryptopp/blob/asan-esign-test/tests/esign_memcpy_asan.cpp The test validates 17 seed lengths (including 0) across 3 modulus sizes for comprehensive edge case coverage. *Evidence* Commit demonstrating the issue: - https://github.com/scc-tw/cryptopp/commit/f47507e6beb94c97cdda84f0d0d06f4299cc51d9 - CI failure: https://github.com/scc-tw/cryptopp/actions/runs/19327409311/job/55281830058 Commit with UBSan fix: - https://github.com/scc-tw/cryptopp/commit/db3951aa00258c2789c2eab519b312de6157f948 - CI success: https://github.com/scc-tw/cryptopp/actions/runs/19327600421/job/55282868767 Complete fix (UBSan + Issue #1338): - https://github.com/scc-tw/cryptopp/commit/ab1878dde7658ff8f18482d9944f6996bf3155a6 - CI validation: https://github.com/scc-tw/cryptopp/actions/runs/19327831662/job/55283667912 *Pull Request #1340 <https://github.com/weidai11/cryptopp/pull/1340>* I've opened Pull Request #1340 with this fix: https://github.com/weidai11/cryptopp/pull/1340 The fix applies three changes to esign.cpp:117-122: ```cpp seed.resize(seedParam.size() + 4); // Help static analyzer verify bounds (Issue #1338) CRYPTOPP_ASSERT(seed.size() >= seedParam.size() + 4); // Guard against null pointer when size is 0 (UBSan) if (seedParam.size() > 0) std::memcpy(seed.data() + 4, seedParam.begin(), seedParam.size()); ``` *Key changes:* 1. Use seed.data() instead of seed + 4: Clearer for static analyzers, resolves clang22 fortify warning 2. Add CRYPTOPP_ASSERT: Explicit bounds verification helps compiler verify safety 3. Guard with if (size > 0): Prevents null pointer UB, handles empty seed edge case This approach is inspired by the cryptopp-modern fork fix (commit 79ecd1f0 <https://github.com/Coralesoft/cryptopp-modern/commit/79ecd1f0>) with additional UBSan protection. *Testing* The fix has been validated with: - ✅ ASan/UBSan (no violations) - ✅ Clang22 -O3 with fortify (no warnings) - ✅ Full cryptest.exe validation suite - ✅ 51 ESIGN edge cases (0-128 byte seeds, 96-384 bit moduli) *Questions* 1. Is the fix approach acceptable? (seed.data() + assert + size guard) 2. Would you like CI validation infrastructure (ASan/UBSan + clang22 fortify) added to the repository? 3. Any additional testing or changes needed for the PR? I'm happy to make any adjustments based on your feedback. Thank you for maintaining this excellent library! Best regards, scc -- *scc 楊志璿 **| Software Engineer* *奧 義 智 慧 科 技* *E* [email protected] *T* +886-2-7739-0077 *F* +886-2-7739-0079 新北市板橋區遠東路3號6樓 Official Site: CyCraft.com <http://www.cycraft.com/> -- You received this message because you are subscribed to the Google Groups "Crypto++ Users" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion visit https://groups.google.com/d/msgid/cryptopp-users/CADzyQwhgEWH8gWwUxCG-hPKe3eenXzyJTqxZoixMZ0-taAWz2Q%40mail.gmail.com.
