fix(cesr): strip identifier bits before reconstructing long-form count in decode_count#313
Open
ManthanNimodiya wants to merge 1 commit intoopenwallet-foundation-labs:mainfrom
Conversation
…t in decode_count Signed-off-by: ManthanNimodiya <manthannimodiya989898@gmail.com>
Contributor
|
Wait a moment, let me check the definition of this part in the spec first. |
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
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.
Summary
decode_counthas a bug in its long-form path (counts ≥ 4096). After matchingthe 6-byte header, it reconstructs the count as
index << 24 | next— butindexis the raw lower-12-bits of the header word, which encodes(identifier << 6) | count_high_6. The identifier bits are never stripped, sothe reconstructed count is wrong for any non-zero identifier. In debug builds
this panics (u32 overflow); in release builds it silently returns a garbage value.
Any TSP message whose encoded payload or envelope exceeds ~12 KB triggers the
long-form path and will fail to decode correctly.
Root Cause
tsp_sdk/src/cesr/decode.rs, functiondecode_count, long-form branch:The encoder (
encode_count) correctly packscount >> 24into the low 6 bitsof the header word. The decoder needs to extract exactly those 6 bits before
shifting, not the full 12-bit
index.Changes
tsp_sdk/src/cesr/decode.rs— one-character fix:index << 24→(index & 0x3F) << 24tsp_sdk/src/cesr/mod.rs— addeddecode_count_long_form_round_tripstest covering all four TSP framing identifiers (ETS_WRAPPER, HOP_LIST,
S_WRAPPER, PAYLOAD) at the exact long-form boundary (4096) and at larger
values, verifying encode→decode round-trips correctly and the stream is
fully consumed
Testing
cargo test -p tsp_sdkNew test:
decode_count_long_form_round_trips— 8 cases covering all realTSP framing identifiers in both boundary and large-count scenarios.
Checklist