fix: escape relying party text in locally built consent messages - #895
Draft
AntonioVentilii wants to merge 1 commit into
Draft
fix: escape relying party text in locally built consent messages#895AntonioVentilii wants to merge 1 commit into
AntonioVentilii wants to merge 1 commit into
Conversation
The memo is rendered as an inline code span and the token symbol is Markdown escaped, so that neither can carry formatting or markup into the message a wallet displays for approval.
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.
Motivation
When a targeted ledger does not implement ICRC-21, the library builds the consent message itself, and values the relying party controls were interpolated in that Markdown as they are. The ICRC-1 memo is arbitrary bytes decoded to UTF-8, so it can carry Markdown formatting or HTML markup into the message a wallet renders for approval. A payload as short as
<button style=zoom:99>fits in the 32 byte memo every conforming ledger accepts, and in a wallet rendering the message as HTML it becomes a control on the approval screen. The token symbol, read from the canister the relying party asks to call, is chosen by it too.Escaping the angle brackets alone would not be enough: Markdown misleads on its own with headings, links, emphasis, or a line impersonating a field the wallet displays.
Changes
src/utils/markdown.utils.ts.inlineCoderenders a text as an inline code span, delimited by more backticks than the longest run it contains so it cannot close its own span.escapeMarkdownbackslash escapes the characters Markdown gives a meaning to, angle brackets included. Both replace control characters, line breaks and invisible formatting characters with a space.buildMemo, shared by theicrc1_transfer,icrc2_approveandicrc2_transfer_frombuilders.Amounts, dates and accounts come from decoded numbers or
encodeIcrcAccount, so they cannot carry syntax. Still possible: the memo is displayed, so it can hold misleading wording, look alike glyphs or a plain text URL, and bounding its length stays a wallet concern.Tests
Unit tests for both helpers, and per builder a memo with markup, one with Markdown, one impersonating a section, and a symbol with markup. Plain text memos display as before. Ran format, lint, test and build, all passing.