Zondax / ledger-stacks

Apache License 2.0
19 stars 7 forks source link

Message signing: payloads not accepted, line break issues #137

Closed kyranjamie closed 1 year ago

kyranjamie commented 1 year ago

See https://github.com/hirosystems/stacks-wallet-web/issues/2725 on Hiro Wallet repo.

We've noticed some instances where the Ledger app will reject payloads. One example is on Gamma.io. Follow the auth flow, which triggers a message signing request to repro.

They try and sign this payload, which is rejected

Welcome!
Sign this message to access Gamma's full feature set.
As always, by using Gamma, you agree to our terms of use: https://gamma.io/terms
Domain: gamma.io
Account: SP2PH3XAPDMSKXQVS1WZ80JGZACY713JQQEE1DY48
Nonce: c83024f9e9aef40f5d72076e883054c07100035112826b14f78e5a893d62b1bf`

Further, the screen preview crops some message contents. This appears to surface when there are line breaks: test message

const testPayload = `one\ntwo\nthree\nfour\nfive\nsix\nseven\neight\nnine\nten\neleven\ntwelve\nthirteen\nfourteen\nfiveteen\nsixteen`;
Demo photos ![IMG_0426](https://user-images.githubusercontent.com/1618764/196128189-39f5e38d-8e85-4962-a638-2a528cc1d7a8.jpeg) ![IMG_0427](https://user-images.githubusercontent.com/1618764/196128300-62a7f120-35bb-4dbe-878e-87ad74136d1b.jpeg) ![IMG_0428](https://user-images.githubusercontent.com/1618764/196128412-0aa1b79e-f299-488b-9a44-61688def2bb0.jpeg)

Let me know if you need any help reproducing

:link: zboto Link

markmhendrickson commented 1 year ago

Do we have a sense of just what part of this message payload is causing the rejection? Any line breaks whatsoever?

kyranjamie commented 1 year ago

Do we have a sense of just what part of this message payload is causing the rejection? Any line breaks whatsoever?

I tried many combinations: varying message/line lengths, line breaks, non-alphanumeric characters etc, and couldn't establish any hard rules as to where/when it breaks

markmhendrickson commented 1 year ago

The fix has been made here: https://github.com/Zondax/ledger-stacks/pull/138

It's getting submitted to Ledger for review and will need their approval before getting published in Ledger Live.