[CI] next-release#81
Draft
overcat wants to merge 20 commits into
Draft
Conversation
# Conflicts: # Cargo.lock
The handler had no limit on accumulated message length: a ~5 KB message of non-printable bytes escaped to a ~20 KB display string on a 16 KB heap, aborting the app inside the allocator. Cap messages at 2 KB so the escaped form (worst case 4x expansion) stays within budget, and reject oversized input early with RequestDataTooLarge. Also stop copying prefix + message into a temporary Vec just to hash it — feed both parts to SHA-256 incrementally via the new hash_parts — and build the escaped string with a single exact-size allocation instead of collect's realloc growth.
…xt after terminal results A transfer opening with P1_MORE skipped path parsing and proceeded with the default path [0,0,0]; sign_tx then derives the public key before any user interaction, and BOLOS rejects the out-of-manifest path with an app-terminating exception — a one-APDU, no-interaction crash. Stale raw_data also survived completed or errored signs, so a later continuation chunk concatenated onto the previous transaction's bytes under the previous path. Track a chunk_started flag in AppContext and fail continuation chunks with DataParsingFail while it is unset (covers both normal and swap modes via the shared handle_chunk). Reset the context in the main loop after every terminal result — after show_status_and_home_if_needed, which still needs review_finished — keeping it only across successful mid-stream chunks. Well-behaved hosts are unaffected; out-of-order chunks now get a clean 0xB005 instead of state confusion or an app kill.
The "Pool Fee Rate" display used `fee as f64 / 100.0` inside format! — the sole float in the app — pulling soft-float division and the float shortest-roundtrip printer into the opt-level='z' LTO'd binary. Format the percentage with integer math instead, reproducing the f64 Display output exactly (trailing zeros trimmed, sign handled via unsigned_abs). Equivalence is locked by a unit test sweeping -1M..=1M plus i32 edges against the old expression byte-for-byte; on-screen output is unchanged for every possible fee value. Size (flex, release): .text 133,040 -> 123,824 bytes (-9,216, -6.9%).
op_count was an unvalidated attacker-controlled u32; ui_sign_tx renders several heap strings per operation, so a forged count could exhaust the 16 KB heap and abort the app. Reject counts above 100 (the Stellar protocol MAX_OPS_PER_TX) in Transaction::parse, covering both the normal and swap paths. Real-world mainnet transactions in the dataset go up to 49 operations, so no network-valid transaction is refused.
…iation format_decimal, format_native_amount, format_token_amount, and format_amount_with_asset were generic over T: ToString + Copy but immediately stringify, so each caller type (u32, i64, u64, i128) duplicated the full decimal/comma logic in the binary. Take a concrete i128 (i64 for the private asset helper) instead — every amount type in the app and every on-chain Stellar amount type converts losslessly, and the exhaustive i128 tests now cover the one production instantiation. Output is unchanged for all values. Size (flex, release): .text 123,824 -> 123,312 bytes (-512).
…ting The manual timestamp algorithm was fuzz-verified against Python datetime during review, but only 3 values were locked in tests. Add leap day, Feb 28/Mar 1 transitions in leap and non-leap years, day 366, the Gregorian century rule (2000 leap, 2100 not), and both sides of the 253402300799 cutoff, so regressions in the leap logic can't land silently. Expected values computed with Python datetime (UTC).
…Fail The three call sites are currently infallible (input is always the internal 32-byte get_public_key output), but in a panic-aborts-the-app codebase an .expect() is a latent crash point for any future refactor that passes externally-sized data. Return AppSW::KeyDeriveFail instead; behavior today is unchanged.
…hange contract The fallback returned "0", which Exchange would display as a real amount. The documented contract (and every other app: the C version of this app, app-solana, app-xrp) sets an empty printable_amount to signal failure. The path is unreachable today, so behavior is unchanged.
The swap validator parsed exactly one operation and returned, while the whole raw buffer gets hashed and signed — bytes after the operation (a non-void ext carrying SorobanTransactionData, or arbitrary trailing garbage) were covered by the signature but never validated. Require a void ext (0) after the operation and full buffer consumption, restoring the sign-only-what-was-validated invariant on the no-UI swap path. Well-formed Exchange payments end with exactly the 4-byte void ext, so the legitimate flow is unaffected.
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
1 participant
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.
Checklist
develop