LedgerHQ / wallet-connect-live-app

https://wallet-connect-live-app.vercel.app/
7 stars 5 forks source link

Wallet Connect 2 wrong signature for hex message #121

Closed gvoltr closed 1 month ago

gvoltr commented 11 months ago

Greetings! While testing WC with ledger I found some weird behavior. Most of the messages we are passing from our app to the ledger are signed fine but signature of hex strings is not correct.

For instance I'm trying to sign this string "0x1e856e530dcc8ba13b362d6a43eeb104fda6cbfbe465fdf13d004e06ce12ba8d". In ledger app I see this hex converted to some invalid string.

Screenshot_2023-09-14-16-01-52-002_com ledger live

On the ledger device I see message in different format

IMG_0560

After signing this on ledger device I do get result in our app but, the signature is invalid. This exact flow works fine with other wallets. Anything I can do from our app side to fix this issue?

github-actions[bot] commented 10 months ago

This issue is stale because it has been open 30 days with no activity. Remove stale label, comment, or consider closing it.

gvoltr commented 10 months ago

I believe this issue still exists, can we reopen @mcayuelas-ledger ?

github-actions[bot] commented 9 months ago

This issue is stale because it has been open 30 days with no activity. Remove stale label, comment, or consider closing it.

PaulUD commented 2 months ago

@mcayuelas-ledger this issue is still present. Can we please reopen, thanks.

mcayuelas-ledger commented 2 months ago

@Justkant @ComradeAERGO

RamyEB commented 1 month ago

Hi ! Thank you for your feedback @gvoltr.

Can you provide more details on which other wallets work well? What is happening for now is that we decode every messages and it seems that your hex string refers to invalid UTF-8 characters. You can encode your string "0x1e856e530dcc8ba13b362d6a43eeb104fda6cbfbe465fdf13d004e06ce12ba8d" to hex for now, while we investigate how other wallets work.

import * as encoding from "@walletconnect/encoding";
encoding.utf8ToHex(message, true);

Ramy

IMG_8886
gvoltr commented 1 month ago

Hello! Thanks for the response! We have 2 types of messages that our app signs.

I just rechecked this with Metamask and Trust wallets.

For UI part wallets seems to check if hex is a valid UTF-8 then readable string is displayed, if not - raw hex message is displayed. Both wallets produce correct signature for the second type of message.

In our app when we sign with a private key, both types works as well. Here is the example. This code uses org.web3j library.

val data = Numeric.hexStringToByteArray(message)
val signature = Sign.signPrefixedMessage(data, keyPair)
RamyEB commented 1 month ago

Hi @gvoltr,

Thank you for your feedback.

After conducting severals tests on other wallets, we have concluded that we will not integrate a display adaptation feature based on the received message for more clarity and transparency for the end user.

We recommend using a method for encoding valid UTF-8 values in hex, as advised earlier

import * as encoding from "@walletconnect/encoding";
encoding.utf8ToHex(message, true);

or in the Metamask documentation.