Fix 'Assertion failed' in softhsm2-dump-file when encountering a zero-length element#761
Fix 'Assertion failed' in softhsm2-dump-file when encountering a zero-length element#761dogo42 wants to merge 4 commits intosofthsm:mainfrom
Conversation
…_SUBJECT is in imported keypairs Example output: 00 00 00 00 00 00 01 01 CKA_SUBJECT 00 00 00 00 00 00 00 03 byte string attribute 00 00 00 00 00 00 00 00 (length 0) /usr/include/c++/11/bits/stl_vector.h:1045: std::vector<_Tp, _Alloc>::reference std::vector<_Tp, _Alloc>::operator[](std::vector<_Tp, _Alloc>::size_type) [with _Tp = unsigned char; _Alloc = std::allocator<unsigned char>; std::vector<_Tp, _Alloc>::reference = unsigned char&; std::vector<_Tp, _Alloc>::size_type = long unsigned int]: Assertion '__n < this->size()' failed.
…e-fix Fix 'Assertion failed' when encountering zero-length element like CKA…
Co-authored-by: Björn Svensson <bjorn.a.svensson@est.tech>
Co-authored-by: Björn Svensson <bjorn.a.svensson@est.tech>
WalkthroughThe changes focus on improving the attribute handling in the Changes
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/bin/dump/softhsm2-dump-file.cpp (2)
440-448: LGTM! The fix prevents assertion failures for zero-length elements.The changes correctly handle zero-length elements by avoiding unnecessary vector allocation.
However, the brace style needs to be adjusted to match the project's conventions.
- if (len > 0) - { + if (len > 0) + {
467-476: LGTM! The fix prevents assertion failures for zero-length elements.The changes correctly handle zero-length elements by avoiding unnecessary vector allocation.
The brace style and indentation need to be adjusted:
- if (len > 0) - { + if (len > 0) + { std::vector<Attribute> value; if (!readMap(stream, len, value)) { corrupt(stream); return; } dumpMap(value); - } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/bin/dump/softhsm2-dump-file.cpp(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Windows (x86, openssl)
- GitHub Check: Windows (x64, botan)
- GitHub Check: macOS (botan)
- GitHub Check: Windows (x64, openssl)
- GitHub Check: Linux (botan)
- GitHub Check: macOS (openssl)
- GitHub Check: Linux (openssl)
🔇 Additional comments (1)
src/bin/dump/softhsm2-dump-file.cpp (1)
Line range hint
440-476: Overall changes look good and effectively fix the issue.The changes consistently handle zero-length elements for both
BYTES_ATTRandATTRMAP_ATTRtypes by:
- Checking the length before allocating vectors
- Only performing operations when length > 0
- Maintaining existing error handling patterns
This effectively fixes the assertion failure mentioned in the PR description while keeping the code maintainable.
bjosv
left a comment
bjosv
left a comment
There was a problem hiding this comment.
LGTM, but with same nitpick comments as coderabbitai
3 participants
softhsm2-dump-file dies when it encounters a zero-length element
like CKA_SUBJECT in imported keypairs, but tries to treat the
element as std::vector.
Example output:
...
00 00 00 00 00 00 01 01 CKA_SUBJECT
00 00 00 00 00 00 00 03 byte string attribute
00 00 00 00 00 00 00 00 (length 0)
/usr/include/c++/11/bits/stl_vector.h:1045: std::vector<_Tp, _Alloc>::reference std::vector<_Tp, _Alloc>::operator[](std::vector<_Tp, _Alloc>::size_type) [with _Tp = unsigned char; _Alloc = std::allocator; std::vector<_Tp, _Alloc>::reference = unsigned char&; std::vector<_Tp, _Alloc>::size_type = long unsigned int]: Assertion '__n < this->size()' failed.
Program received signal SIGABRT, Aborted.
Summary by CodeRabbit