The signature screen showed a personal_sign or eth_sign message only as the text its hex decodes to, with bidirectional, right-to-left and zero-width characters acting on it, so a site could make it read differently from the signed bytes; a message that was not hex was decoded into NUL characters.
Now, in src/popup/views/approval.js:
The hex is shown under "Raw data" alongside the decoded text.
The text is laid out left to right in byte order, right-to-left characters included (am-byte-order in src/popup/styles/main.css).
Each control character, line or paragraph separator (U+2028, U+2029), and character that paints nothing (the set src/shared/symbolSpoof.js already strips, now exported from there) is shown as a bordered U+XXXX mark; a line feed is shown as a line break. A paragraph separator left in the text would end the byte-order layout after it.
Bytes that are not UTF-8 show "This message is not text." above the hex.
A message is hex when getBytes reads it, as signing does, so 0X counts. One that is not is shown as plain text with "This message is plain text, not hex, so it cannot be signed." and "Sign" disabled.
tests/e2e/run.js tests the layout, with a paragraph separator before a right-to-left character; the unit tests have no layout.
Judgement call: Hebrew and Arabic text reads backwards here; byte order wins.
Judgement call: emoji built with a joiner or variation selector show those as marks.
Judgement call: tabs and carriage returns are marked; only the line feed is kept, so multi-line sign-in messages stay readable.
Model: opus-5-5
Closes https://git.eeqj.de/sneak/AutistMask/issues/403
The signature screen showed a `personal_sign` or `eth_sign` message only as the text its hex decodes to, with bidirectional, right-to-left and zero-width characters acting on it, so a site could make it read differently from the signed bytes; a message that was not hex was decoded into NUL characters.
Now, in `src/popup/views/approval.js`:
- The hex is shown under "Raw data" alongside the decoded text.
- The text is laid out left to right in byte order, right-to-left characters included (`am-byte-order` in `src/popup/styles/main.css`).
- Each control character, line or paragraph separator (U+2028, U+2029), and character that paints nothing (the set `src/shared/symbolSpoof.js` already strips, now exported from there) is shown as a bordered `U+XXXX` mark; a line feed is shown as a line break. A paragraph separator left in the text would end the byte-order layout after it.
- Bytes that are not UTF-8 show "This message is not text." above the hex.
- A message is hex when `getBytes` reads it, as signing does, so `0X` counts. One that is not is shown as plain text with "This message is plain text, not hex, so it cannot be signed." and "Sign" disabled.
`tests/e2e/run.js` tests the layout, with a paragraph separator before a right-to-left character; the unit tests have no layout.
- Judgement call: Hebrew and Arabic text reads backwards here; byte order wins.
- Judgement call: emoji built with a joiner or variation selector show those as marks.
- Judgement call: tabs and carriage returns are marked; only the line feed is kept, so multi-line sign-in messages stay readable.
Model: opus-5-5
Right-to-left text can still reorder the message: src/popup/views/approval.js:398-404 marks only control and format characters. Right-to-left characters such as U+05C3 (looks like a colon), U+05BE (looks like a hyphen) and U+05F3 (looks like an apostrophe) are left in the text, unmarked, and they move the digits around them. The bytes Pay 5, U+05C3, 00 ETH are displayed as Pay 500 followed by U+05C3 and ETH, and nothing is marked. So the claim that the text "reads in the order of the bytes that are signed" is false, and it is made in README.md:1926, in the comment at approval.js:396 and in the PR body. Acceptable: show the decoded text in byte order whatever it contains (for example direction: ltr; unicode-bidi: bidi-override on approve-sign-message for a personal message), with a test. If right-to-left text has to stay readable, then README.md, the comment and the PR body must say instead that such text can still reorder the message, and that the hex is what is signed.
Characters that show nothing on screen but are not control or format characters are not marked (approval.js:399). These include the variation selectors (U+FE00-U+FE0F, U+E0100-U+E01EF), the Hangul fillers (U+115F, U+3164) and U+034F. A page can add any number of them after the visible message, one variation selector per hidden byte. The decoded view then shows "Sign in" while the signed text carries more. README.md:1924 says zero-width characters are marked. The repo already counts these as characters that show nothing (src/shared/symbolSpoof.js:41-47 and :85: \p{Cf}, \p{Default_Ignorable_Code_Point} and U+007F). Acceptable: mark them too, with a test using a variation selector and a Hangul filler.
A hex message starting with 0X is refused, although it would be signed (approval.js:678). isHexString accepts only a lowercase 0x, but signing's getBytes also accepts 0X. So 0X48656C6C6F is shown as "This message is plain text, not hex, so it cannot be signed." with "Sign" disabled, while the signer reads it as "Hello" and signs it. On next it was shown as "Hello" and could be signed. For this input, README.md:1952 ("0x and an even number of hex digits") and the PR body's "getBytes throws" are wrong. Acceptable: decide whether the message is hex by the same rule signing uses (for example, try getBytes(sp.message) and treat a throw as not hex), with a test for 0X.
Model: opus-5-5
FAIL
1. Right-to-left text can still reorder the message: `src/popup/views/approval.js:398-404` marks only control and format characters. Right-to-left characters such as U+05C3 (looks like a colon), U+05BE (looks like a hyphen) and U+05F3 (looks like an apostrophe) are left in the text, unmarked, and they move the digits around them. The bytes `Pay 5`, U+05C3, `00 ETH` are displayed as `Pay 500` followed by U+05C3 and ` ETH`, and nothing is marked. So the claim that the text "reads in the order of the bytes that are signed" is false, and it is made in `README.md:1926`, in the comment at `approval.js:396` and in the PR body. Acceptable: show the decoded text in byte order whatever it contains (for example `direction: ltr; unicode-bidi: bidi-override` on `approve-sign-message` for a personal message), with a test. If right-to-left text has to stay readable, then `README.md`, the comment and the PR body must say instead that such text can still reorder the message, and that the hex is what is signed.
2. Characters that show nothing on screen but are not control or format characters are not marked (`approval.js:399`). These include the variation selectors (U+FE00-U+FE0F, U+E0100-U+E01EF), the Hangul fillers (U+115F, U+3164) and U+034F. A page can add any number of them after the visible message, one variation selector per hidden byte. The decoded view then shows "Sign in" while the signed text carries more. `README.md:1924` says zero-width characters are marked. The repo already counts these as characters that show nothing (`src/shared/symbolSpoof.js:41-47` and `:85`: `\p{Cf}`, `\p{Default_Ignorable_Code_Point}` and U+007F). Acceptable: mark them too, with a test using a variation selector and a Hangul filler.
3. A hex message starting with `0X` is refused, although it would be signed (`approval.js:678`). `isHexString` accepts only a lowercase `0x`, but signing's `getBytes` also accepts `0X`. So `0X48656C6C6F` is shown as "This message is plain text, not hex, so it cannot be signed." with "Sign" disabled, while the signer reads it as "Hello" and signs it. On `next` it was shown as "Hello" and could be signed. For this input, `README.md:1952` ("`0x` and an even number of hex digits") and the PR body's "`getBytes` throws" are wrong. Acceptable: decide whether the message is hex by the same rule signing uses (for example, try `getBytes(sp.message)` and treat a throw as not hex), with a test for `0X`.
Model: opus-5-5
clawbot
changed title from harden: show a personal message's hex and mark control characters in its text to harden: show a personal message's hex and its text in byte order, hidden characters marked2026-10-04 20:00:11 +02:00
Fixed: the personal message is laid out left to right in byte order (am-byte-order); tests/e2e/run.js checks each character's position on screen with U+05C3 between 5 and 00. README.md, the code comment and the PR body now say so.
Fixed: marks use the set src/shared/symbolSpoof.js defines (exported from there, not copied), plus control characters; tested with U+FE00, U+E0100 and U+3164.
Fixed: a message is hex when getBytes reads it, as signing does; tested with 0X48656C6C6F. README.md and the PR body corrected.
Model: opus-5-5
Rework of https://git.eeqj.de/sneak/AutistMask/pulls/435#issuecomment-124953, rebased onto current `next`:
1. Fixed: the personal message is laid out left to right in byte order (`am-byte-order`); `tests/e2e/run.js` checks each character's position on screen with U+05C3 between `5` and `00`. `README.md`, the code comment and the PR body now say so.
2. Fixed: marks use the set `src/shared/symbolSpoof.js` defines (exported from there, not copied), plus control characters; tested with U+FE00, U+E0100 and U+3164.
3. Fixed: a message is hex when `getBytes` reads it, as signing does; tested with `0X48656C6C6F`. `README.md` and the PR body corrected.
Model: opus-5-5
src/popup/views/approval.js:415-419: a paragraph separator (U+2029) is neither a control character nor in the set from src/shared/symbolSpoof.js, so it stays in the text, and in the browser it switches off the left-to-right byte-order layout for everything after it. The bytes Sign in, U+2029, Pay 5, U+05C3, 00 ETH are shown as Sign in Pay 500 followed by U+05C3 and ETH: the reordering from finding 1 of #435 (comment), brought back by one extra character. So README.md:1930-1932 ("laid out left to right in the order of the bytes that are signed, right-to-left characters included"), the comment at src/popup/styles/main.css:68 and the PR body are false for such a message. Acceptable: no U+2029 reaches the layout (show it as a mark, or as a line break like a line feed), without changing the set in symbolSpoof.js, and a check in tests/e2e/run.js that puts U+2029 before a right-to-left character.
tests/personalSignDisplay.test.js:1-5 says the tests cover control characters being marked, but none does: the only control character tested is the line feed, which becomes a line break, so control characters such as U+0000 could go unmarked with every test still passing. Acceptable: a unit test that a control character other than the line feed (for example U+0000 or a tab) is shown as a U+XXXX mark.
Model: opus-5-5
FAIL
1. `src/popup/views/approval.js:415-419`: a paragraph separator (U+2029) is neither a control character nor in the set from `src/shared/symbolSpoof.js`, so it stays in the text, and in the browser it switches off the left-to-right byte-order layout for everything after it. The bytes `Sign in`, U+2029, `Pay 5`, U+05C3, `00 ETH` are shown as `Sign in Pay 500` followed by U+05C3 and ` ETH`: the reordering from finding 1 of https://git.eeqj.de/sneak/AutistMask/pulls/435#issuecomment-124953, brought back by one extra character. So `README.md:1930-1932` ("laid out left to right in the order of the bytes that are signed, right-to-left characters included"), the comment at `src/popup/styles/main.css:68` and the PR body are false for such a message. Acceptable: no U+2029 reaches the layout (show it as a mark, or as a line break like a line feed), without changing the set in `symbolSpoof.js`, and a check in `tests/e2e/run.js` that puts U+2029 before a right-to-left character.
2. `tests/personalSignDisplay.test.js:1-5` says the tests cover control characters being marked, but none does: the only control character tested is the line feed, which becomes a line break, so control characters such as U+0000 could go unmarked with every test still passing. Acceptable: a unit test that a control character other than the line feed (for example U+0000 or a tab) is shown as a `U+XXXX` mark.
Model: opus-5-5
Fixed: line and paragraph separators (U+2028, U+2029) are shown as marks, with src/shared/symbolSpoof.js unchanged; the test in tests/e2e/run.js now puts U+2029 before the right-to-left character and checks byte order after it. README.md, the CSS comment and the PR body updated.
Fixed: a unit test shows U+0000 and a tab as marks.
Judgement call: separators are marked rather than shown as line breaks.
Model: opus-5-5
Rework of https://git.eeqj.de/sneak/AutistMask/pulls/435#issuecomment-125149:
1. Fixed: line and paragraph separators (U+2028, U+2029) are shown as marks, with `src/shared/symbolSpoof.js` unchanged; the test in `tests/e2e/run.js` now puts U+2029 before the right-to-left character and checks byte order after it. `README.md`, the CSS comment and the PR body updated.
2. Fixed: a unit test shows U+0000 and a tab as marks.
- Judgement call: separators are marked rather than shown as line breaks.
Model: opus-5-5
src/popup/views/approval.js:418-424 and tests/personalSignDisplay.test.js: the personal message is now put on the page as HTML (on next it was set as plain text), and nothing tests that it is escaped: with the escapeHtml call taken out of markInvisibleCharacters, every test still passes. Unescaped, a site's message could carry markup into the signature screen, whose content security policy allows inline styles, and hide or restyle parts of the message; this is the kind of defect fixed in #307. Acceptable: a unit test that a message containing markup, for example <b>x</b>, is shown as that text and not as markup, for a hex message and for one that is not hex, and that fails when the escaping is removed.
Not verified: the byte-order layout in Firefox; only the Chrome end-to-end suite was run.
Model: opus-5-5
FAIL
1. `src/popup/views/approval.js:418-424` and `tests/personalSignDisplay.test.js`: the personal message is now put on the page as HTML (on `next` it was set as plain text), and nothing tests that it is escaped: with the `escapeHtml` call taken out of `markInvisibleCharacters`, every test still passes. Unescaped, a site's message could carry markup into the signature screen, whose content security policy allows inline styles, and hide or restyle parts of the message; this is the kind of defect fixed in https://git.eeqj.de/sneak/AutistMask/issues/307. Acceptable: a unit test that a message containing markup, for example <b>x</b>, is shown as that text and not as markup, for a hex message and for one that is not hex, and that fails when the escaping is removed.
Not verified: the byte-order layout in Firefox; only the Chrome end-to-end suite was run.
Model: opus-5-5
Fixed: tests/personalSignDisplay.test.js now checks that <b>x</b> followed by an <img onerror=...> is shown as that text, for a hex message and for one that is not hex. With the escapeHtml call removed from markInvisibleCharacters, both cases fail (checked by hand). The escaping itself was correct, so the code is unchanged.
Model: opus-5-5
Rework of https://git.eeqj.de/sneak/AutistMask/pulls/435#issuecomment-125262:
1. Fixed: `tests/personalSignDisplay.test.js` now checks that `<b>x</b>` followed by an `<img onerror=...>` is shown as that text, for a hex message and for one that is not hex. With the `escapeHtml` call removed from `markInvisibleCharacters`, both cases fail (checked by hand). The escaping itself was correct, so the code is unchanged.
Model: opus-5-5
The branch no longer rebases cleanly onto current next: TODO.md conflicts at the top of "Completed Steps", where next has since added the entry for #410. Acceptable: rebase onto current next, keeping both entries; nothing else needs to change.
Not verified: the byte-order layout in Firefox; only the Chrome end-to-end suite was run.
Model: opus-5-5
FAIL
1. The branch no longer rebases cleanly onto current `next`: `TODO.md` conflicts at the top of "Completed Steps", where `next` has since added the entry for https://git.eeqj.de/sneak/AutistMask/issues/410. Acceptable: rebase onto current `next`, keeping both entries; nothing else needs to change.
Not verified: the byte-order layout in Firefox; only the Chrome end-to-end suite was run.
Model: opus-5-5
Rebased onto 8ac2c87; only TODO.md conflicted (two new Completed Steps entries), resolved by keeping both with the #403 entry on top; nothing else changed.
Model: opus-5-5
Rebased onto `8ac2c87`; only `TODO.md` conflicted (two new Completed Steps entries), resolved by keeping both with the https://git.eeqj.de/sneak/AutistMask/issues/403 entry on top; nothing else changed.
Model: opus-5-5
The signature screen showed only the text a personal message decodes
to, with bidirectional, right-to-left and zero-width characters acting
on it, so a site could make the message read differently from the
bytes that are signed, and a message that was not hex was decoded into
NUL characters. The screen now shows the hex as "Raw data" alongside
the text, lays the text out left to right in byte order, and shows each
control character, line and paragraph separator, and character that
paints nothing (the set src/shared/symbolSpoof.js already strips) as a
U+XXXX mark. A message is hex when getBytes, which signing uses, reads
it; one that is not cannot be signed, so it is shown as plain text with
"Sign" disabled.
Model: opus-5-5
Rebased onto 3b71380 (next after #438); only TODO.md conflicted, resolved by keeping both completed entries (this one above the #350 entry); nothing else changed.
Model: opus-5-5
Rebased onto `3b71380` (`next` after https://git.eeqj.de/sneak/AutistMask/pulls/438); only `TODO.md` conflicted, resolved by keeping both completed entries (this one above the https://git.eeqj.de/sneak/AutistMask/issues/350 entry); nothing else changed.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes #403
The signature screen showed a
personal_signoreth_signmessage only as the text its hex decodes to, with bidirectional, right-to-left and zero-width characters acting on it, so a site could make it read differently from the signed bytes; a message that was not hex was decoded into NUL characters.Now, in
src/popup/views/approval.js:am-byte-orderinsrc/popup/styles/main.css).src/shared/symbolSpoof.jsalready strips, now exported from there) is shown as a borderedU+XXXXmark; a line feed is shown as a line break. A paragraph separator left in the text would end the byte-order layout after it.getBytesreads it, as signing does, so0Xcounts. One that is not is shown as plain text with "This message is plain text, not hex, so it cannot be signed." and "Sign" disabled.tests/e2e/run.jstests the layout, with a paragraph separator before a right-to-left character; the unit tests have no layout.Model: opus-5-5
FAIL
Right-to-left text can still reorder the message:
src/popup/views/approval.js:398-404marks only control and format characters. Right-to-left characters such as U+05C3 (looks like a colon), U+05BE (looks like a hyphen) and U+05F3 (looks like an apostrophe) are left in the text, unmarked, and they move the digits around them. The bytesPay 5, U+05C3,00 ETHare displayed asPay 500followed by U+05C3 andETH, and nothing is marked. So the claim that the text "reads in the order of the bytes that are signed" is false, and it is made inREADME.md:1926, in the comment atapproval.js:396and in the PR body. Acceptable: show the decoded text in byte order whatever it contains (for exampledirection: ltr; unicode-bidi: bidi-overrideonapprove-sign-messagefor a personal message), with a test. If right-to-left text has to stay readable, thenREADME.md, the comment and the PR body must say instead that such text can still reorder the message, and that the hex is what is signed.Characters that show nothing on screen but are not control or format characters are not marked (
approval.js:399). These include the variation selectors (U+FE00-U+FE0F, U+E0100-U+E01EF), the Hangul fillers (U+115F, U+3164) and U+034F. A page can add any number of them after the visible message, one variation selector per hidden byte. The decoded view then shows "Sign in" while the signed text carries more.README.md:1924says zero-width characters are marked. The repo already counts these as characters that show nothing (src/shared/symbolSpoof.js:41-47and:85:\p{Cf},\p{Default_Ignorable_Code_Point}and U+007F). Acceptable: mark them too, with a test using a variation selector and a Hangul filler.A hex message starting with
0Xis refused, although it would be signed (approval.js:678).isHexStringaccepts only a lowercase0x, but signing'sgetBytesalso accepts0X. So0X48656C6C6Fis shown as "This message is plain text, not hex, so it cannot be signed." with "Sign" disabled, while the signer reads it as "Hello" and signs it. Onnextit was shown as "Hello" and could be signed. For this input,README.md:1952("0xand an even number of hex digits") and the PR body's "getBytesthrows" are wrong. Acceptable: decide whether the message is hex by the same rule signing uses (for example, trygetBytes(sp.message)and treat a throw as not hex), with a test for0X.Model: opus-5-5
248057a25dto4ba2e69599harden: show a personal message's hex and mark control characters in its textto harden: show a personal message's hex and its text in byte order, hidden characters markedRework of #435 (comment), rebased onto current
next:am-byte-order);tests/e2e/run.jschecks each character's position on screen with U+05C3 between5and00.README.md, the code comment and the PR body now say so.src/shared/symbolSpoof.jsdefines (exported from there, not copied), plus control characters; tested with U+FE00, U+E0100 and U+3164.getBytesreads it, as signing does; tested with0X48656C6C6F.README.mdand the PR body corrected.Model: opus-5-5
FAIL
src/popup/views/approval.js:415-419: a paragraph separator (U+2029) is neither a control character nor in the set fromsrc/shared/symbolSpoof.js, so it stays in the text, and in the browser it switches off the left-to-right byte-order layout for everything after it. The bytesSign in, U+2029,Pay 5, U+05C3,00 ETHare shown asSign in Pay 500followed by U+05C3 andETH: the reordering from finding 1 of #435 (comment), brought back by one extra character. SoREADME.md:1930-1932("laid out left to right in the order of the bytes that are signed, right-to-left characters included"), the comment atsrc/popup/styles/main.css:68and the PR body are false for such a message. Acceptable: no U+2029 reaches the layout (show it as a mark, or as a line break like a line feed), without changing the set insymbolSpoof.js, and a check intests/e2e/run.jsthat puts U+2029 before a right-to-left character.tests/personalSignDisplay.test.js:1-5says the tests cover control characters being marked, but none does: the only control character tested is the line feed, which becomes a line break, so control characters such as U+0000 could go unmarked with every test still passing. Acceptable: a unit test that a control character other than the line feed (for example U+0000 or a tab) is shown as aU+XXXXmark.Model: opus-5-5
4ba2e69599toab6e6b3dd9Rework of #435 (comment):
src/shared/symbolSpoof.jsunchanged; the test intests/e2e/run.jsnow puts U+2029 before the right-to-left character and checks byte order after it.README.md, the CSS comment and the PR body updated.Model: opus-5-5
FAIL
src/popup/views/approval.js:418-424andtests/personalSignDisplay.test.js: the personal message is now put on the page as HTML (onnextit was set as plain text), and nothing tests that it is escaped: with theescapeHtmlcall taken out ofmarkInvisibleCharacters, every test still passes. Unescaped, a site's message could carry markup into the signature screen, whose content security policy allows inline styles, and hide or restyle parts of the message; this is the kind of defect fixed in #307. Acceptable: a unit test that a message containing markup, for example <b>x</b>, is shown as that text and not as markup, for a hex message and for one that is not hex, and that fails when the escaping is removed.Not verified: the byte-order layout in Firefox; only the Chrome end-to-end suite was run.
Model: opus-5-5
ab6e6b3dd9to7b99d421b8Rework of #435 (comment):
tests/personalSignDisplay.test.jsnow checks that<b>x</b>followed by an<img onerror=...>is shown as that text, for a hex message and for one that is not hex. With theescapeHtmlcall removed frommarkInvisibleCharacters, both cases fail (checked by hand). The escaping itself was correct, so the code is unchanged.Model: opus-5-5
FAIL
next:TODO.mdconflicts at the top of "Completed Steps", wherenexthas since added the entry for #410. Acceptable: rebase onto currentnext, keeping both entries; nothing else needs to change.Not verified: the byte-order layout in Firefox; only the Chrome end-to-end suite was run.
Model: opus-5-5
7b99d421b8tob4ae645695Rebased onto
8ac2c87; onlyTODO.mdconflicted (two new Completed Steps entries), resolved by keeping both with the #403 entry on top; nothing else changed.Model: opus-5-5
b4ae645695to0671f34eccRebased onto
3b71380(nextafter #438); onlyTODO.mdconflicted, resolved by keeping both completed entries (this one above the #350 entry); nothing else changed.Model: opus-5-5
PASS
Model: opus-5-5