leather-io / extension

Leather browser extension
https://leather.io
MIT License
294 stars 140 forks source link

Fix Ledger error for Stacks contract executions ("Your ledger device has rejected the payload stating it is invalid") #5115

Open 314159265359879 opened 6 months ago

314159265359879 commented 6 months ago

Reproduction steps

  1. Go to app.arkadiko.finance
  2. Sign in with Leather extension and Ledger device connected
  3. Go to Borrow
  4. Click any of the three options, for example xBTC, click the borrow button.
  5. Input amount of xBTC to collateralize
  6. mint a minimum of 500 USDA
  7. Click continue
  8. Click open vault
  9. Leather popup appears, click confirm
  10. See error image

A user reported that the same thing happens with other vault operations.

Experiencing Issues with Deposit, Withdrawal, and Debt Repayment Interactions

It wasn't an issue prior to this vault v2 merge on Arkadiko's side https://github.com/arkadiko-dao/arkadiko/pull/557

314159265359879 commented 6 months ago

This was tested with Ledger nano S plus (firmware v1.1.1), And Ledger Nano X, with latest firmware.

Leather version 6.30.1 and 6.31.0

Polybius93 commented 6 months ago

This might be related to this issue? https://github.com/leather-wallet/extension/issues/5113

314159265359879 commented 6 months ago

Thanks for sharing! 5113 looks related to bitcoin/PSBT operations, while this is related to Stacks network operations. They are likely unrelated.

314159265359879 commented 6 months ago

That said we do want to do a full audit of the wallet with Ledger this week.

314159265359879 commented 5 months ago

Possibly resolved with #2725?

Retested today Closing vault works with Ledger device. But opening a vault is not supported, "rejected payload..."

Deposit and withdraw, I tested and confirmed to fail with "rejected payload" error too.

DeeList commented 4 months ago

Users are reporting this same issue when visiting Gamma Stacks NFT marketplace and trying to use Ledger.

I've tested on Gamma and confirmed the error.

'There's a technical issue when we try to access our account or manage collection using Leather (connected to Ledger Nano X) on PC. When we click on 'sign message,' on Gamma profile page, it returns this error: 'Your Ledger device has rejected the payload, stating it is invalid.' It occurs only on Stacks. The Bitcoin Gamma it's ok. Tested with: Chrome: v.124.0.6367.63, Leather: v.6.36, Ledger Live: v.2.79.1, Windows 10 Pro: v.22H2'

philiphacks commented 4 months ago

Also on stackingdao btw

pete-watters commented 4 months ago

Thanks for the reports and apologies this isn't working.

I'm doing a full audit of ledger with Leather right now and and will investigate this soon

philiphacks commented 4 months ago

@pete-watters thanks!

the same error also happens when trying to generate a signature on https://app.stackingdao.com/signer?pool=SP4SZE494VC2YC5JYG7AYFQ44F5Q4PYV7DVMDPBG.stacking-pool-v1 - would be great if you can try it (just click on either of the "Save Commit Signature" buttons and sign the message)

image

pete-watters commented 4 months ago

Thanks @philiphacks . I took a quick look to triage and I can indeed re-reproduce the error.

We are showing this error as were are getting a 27012 error message when running whenSignableMessageOfType here. This gives us the response:

{returnCode: 27012, errorMessage: 'Data is invalid : Unexpected data type’}

I also see in the console of https://app.stackingdao.com/ that we are displaying a deprecation warning and recommending to instead use LeatherProvider.request('stx_signMessage')

inpage.ts:257 Deprecation warning: Use LeatherProvider.request('stx_signMessage') instead

I was reproducing and trying to gather more information to see whats going wrong but now when I am clicking Save Commit Signature I am getting a weird disconnection message Screenshot 2024-05-02 at 06 57 08

Have you seen that?

I am not sure if the above is helpful and I will keep investigating but just sharing these initial findings

pete-watters commented 4 months ago

@philiphacks - did you have a chance to check https://github.com/leather-wallet/extension/issues/5115#issuecomment-2089641125?

According to the issue description it seems like this was working prior to https://github.com/arkadiko-dao/arkadiko/pull/557 so perhaps it's not an issue with Leather?

markmhendrickson commented 3 months ago

This has also been reported by ALEX with Leather version 6.41.1 👇

Hi, I am unable to vote on the Treasury Grant Program, and the Re-opening of Unaffected Pools ballots using a Ledger verified account. I see several others above are also getting the same error, but there's no resolution, update, or workaround provided that I can find. Can you advise? The error received is Data Invalid Your Ledger device has rejected the payload stating it is invalid "

philiphacks commented 3 months ago

I'll have a look at this again but afaik this is not Arkadiko-related, I will try to upgrade our dependencies though

314159265359879 commented 3 months ago

Another team experienced similar issues on certain contract calls. And they also ran into this error.

They tried on Xverse and they also get an error. Trying to open a vault with Xverse following the steps in the first post here results in this error: image

I think the issue is on the Ledger/Stacks App side.

The Stacks Foundation will need to be involved to get the Stacks App updated. They can ask Zondax for help.

markmhendrickson commented 2 months ago

@pete-watters has filed here for resolution with the Stacks app, since it appears that the functionality is broken in Leather as a downstream effect of a Ledger-side bug.

pete-watters commented 2 months ago

Zondax replied on their issue and said that there are many changes in https://github.com/Zondax/ledger-stacks/pull/158 and https://github.com/Zondax/ledger-stacks/issues/156 might solve those issues. It is possible to test them in a real device using the app/pkg/nanos@xx.sh installers

They asked:

I will test this and gather the information needed soon but if anyone here already has it to hand please comment and I can relay it to them.

pete-watters commented 2 months ago

I am working with Zondax to try and get to the bottom of this. I have provided them with details of how to replicate the problem on Stacking Dao. I confirmed that the problem does also occur on nano s.

I provided them with the payload we are sending when following these steps:

They replied to say:

This issue would not be fixed by any of the latest changes that will soon be merged. it is likely that the number of nested values in that structured message is triggering a recursion limit in the app, to avoid consuming the stack that is very limited on nanos/s2/x or an unexpected data type in the message domain or while traversing the msg values. there is no way to get the serialized data being send to the ledger? I can not tell with certainty that this is the case. i will take a look at the link and see if I can extract some data to use also in our testing suite

I will try and get the serialised data sent to help them diagnose further

pete-watters commented 2 months ago

I passed on some more information to Zondax: https://github.com/Zondax/ledger-stacks/issues/159#issuecomment-2223276545

They said:

I will put it in our tests suite internally for easy debugging of this bug, hopefully, it would be about adjusting some types/value limits and not due to device limitations. I will keep you posted

neithanmo commented 1 month ago

Hi @all We were provided the following data that in theory would help to reproduce the issue in discussion here:

However, that data passed our tests and we do not see any issue,

As we are preparing for a release, I would rather want to revisit this in order to apply any fixes if necessary, can anyone help by providing similar information as above that we can use to reproduce the issue?

Dior-Aranel commented 1 month ago

@neithanmo Is there an idiot's guide to finding the domain and payload?

My derivation path in Ledger is the same as yours, but I'm still getting the error regardless, when I try to open an Arkadiko vault. I'd be happy to provide my data, but I'm not sure how to find those two

Leather version 6.44.0 Ledger OS version 2.2.4

pete-watters commented 1 month ago

Hi @Dior-Aranel ,

I obtained the payload and domain by:

You could add a breakpoint to get that information or else I can get it. You are trying to open a Vault right? Similar to https://github.com/leather-io/extension/issues/5115#issue-2202850199?

I think it could be worth it to update to stx_signMessage as per https://github.com/leather-io/extension/issues/5115#issuecomment-2089641125 however we can see that performing the same action on Xverse also fails https://github.com/leather-io/extension/issues/5115#issuecomment-2145521615

pete-watters commented 1 month ago

I have gathered info reproducing this bug when trying to create a Vault in Arkadiko.

When creating a vault it's slightly different and we use app.sign rather than app.sign_structured_msg. Ledger rejects it with the following error:

errorMessage: "Data is invalid : Authorization type not supported"
returnCode: 27012

For app.sign we don't send the domain but we send this payload:

0000000001040025a6b5c2c50a2abbff700b87c8dfe29bda77523b0000000000000003000000000000e41800000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000003020000000101021625a6b5c2c50a2abbff700b87c8dfe29bda77523b16dbd1c48f77bf2f9506a3d79117fc1c7eda3b89100f577261707065642d426974636f696e0f777261707065642d626974636f696e05000000000015e3070216982f3ec112a5f5928a5c96a914bd733793b896a51f61726b6164696b6f2d7661756c74732d6f7065726174696f6e732d76312d320a6f70656e2d7661756c740000000b0616982f3ec112a5f5928a5c96a914bd733793b896a51b61726b6164696b6f2d7661756c74732d746f6b656e732d76312d310616982f3ec112a5f5928a5c96a914bd733793b896a51961726b6164696b6f2d7661756c74732d646174612d76312d310616982f3ec112a5f5928a5c96a914bd733793b896a51b61726b6164696b6f2d7661756c74732d736f727465642d76312d310616982f3ec112a5f5928a5c96a914bd733793b896a52061726b6164696b6f2d7661756c74732d706f6f6c2d6163746976652d76312d310616982f3ec112a5f5928a5c96a914bd733793b896a51c61726b6164696b6f2d7661756c74732d68656c706572732d76312d310616982f3ec112a5f5928a5c96a914bd733793b896a51461726b6164696b6f2d6f7261636c652d76322d330616dbd1c48f77bf2f9506a3d79117fc1c7eda3b89100f577261707065642d426974636f696e010000000000000000000000000015e307010000000000000000000000001ddca7400a0516c9a03720732a3148129a121f58d2ba37f6c649450100000000000000000000000000000000

When calling app.sign_structured_msg. Ledger rejects it with the following error:

errorMessage: "Data is invalid : Unexpected data type"
returnCode: 27012
Dior-Aranel commented 1 month ago

@pete-watters

Correct, I am trying to open a vault similar to the comment you linked

I tried going through the steps you listed here

The error message I got was: "Your Ledger device has rejected the payload stating it is invalid". When I try to open an Arkadiko vault, that's the same error message I get

I'm still stuck on trying find the domain and payload. I'm a newbie when it comes to digging through Chrome DevTools

neithanmo commented 1 month ago

I have gathered info reproducing this bug when trying to create a Vault in Arkadiko.

When creating a vault it's slightly different and we use app.sign rather than app.sign_structured_msg. Ledger rejects it with the following error:


errorMessage: "Data is invalid : Authorization type not supported"
returnCode: 27012

This is a structured message, so You should use app.sign_structured_msg method, which takes care of construction the header + domain and message(payload)


When calling `app.sign_structured_msg`. Ledger rejects it with the following error:

that is weird, i used your data in a test that calls app.sign_structured_msg:


const signatureRequest = app.sign_structured_msg(
path,
'0c0000000308636861696e2d69640100000000000000000000000000000001046e616d650d0000000c706f782d342d7369676e65720776657273696f6e0d00000005312e302e30',
'0c0000000607617574682d696401000000000000000000000000000ca5640a6d61782d616d6f756e7401000000000000003635c9acdd09faf00006706572696f64010000000000000000000000000000000108706f782d616464720c000000020968617368627974657302000000142fffa9a09bb7fa7dced44834d77ee81c49c5f0cc0776657273696f6e0200000001040c7265776172642d6379636c65010000000000000000000000000000005905746f7069630d0000000a6167672d636f6d6d6974',
)
  // Wait until we are not in the main menu
  await sim.waitUntilScreenIsNot(sim.getMainMenuSnapshot())
  await sim.compareSnapshotsAndApprove('.', `${m.prefix.toLowerCase()}-sign-structured_data_${name}`)

  // Check the signature
  const signature = await signatureRequest

  console.log(signature)
  expect(signature.returnCode).toEqual(0x9000)
  expect(signature.errorMessage).toEqual('No errors')

no [errors](https://github.com/Zondax/ledger-stacks/issues/159#issuecomment-2223331831)
neithanmo commented 1 month ago

I would give this payload:

0000000001040025a6b5c2c50a2abbff700b87c8dfe29bda77523b0000000000000003000000000000e41800000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000003020000000101021625a6b5c2c50a2abbff700b87c8dfe29bda77523b16dbd1c48f77bf2f9506a3d79117fc1c7eda3b89100f577261707065642d426974636f696e0f777261707065642d626974636f696e05000000000015e3070216982f3ec112a5f5928a5c96a914bd733793b896a51f61726b6164696b6f2d7661756c74732d6f7065726174696f6e732d76312d320a6f70656e2d7661756c740000000b0616982f3ec112a5f5928a5c96a914bd733793b896a51b61726b6164696b6f2d7661756c74732d746f6b656e732d76312d310616982f3ec112a5f5928a5c96a914bd733793b896a51961726b6164696b6f2d7661756c74732d646174612d76312d310616982f3ec112a5f5928a5c96a914bd733793b896a51b61726b6164696b6f2d7661756c74732d736f727465642d76312d310616982f3ec112a5f5928a5c96a914bd733793b896a52061726b6164696b6f2d7661756c74732d706f6f6c2d6163746976652d76312d310616982f3ec112a5f5928a5c96a914bd733793b896a51c61726b6164696b6f2d7661756c74732d68656c706572732d76312d310616982f3ec112a5f5928a5c96a914bd733793b896a51461726b6164696b6f2d6f7261636c652d76322d330616dbd1c48f77bf2f9506a3d79117fc1c7eda3b89100f577261707065642d426974636f696e010000000000000000000000000015e307010000000000000000000000001ddca7400a0516c9a03720732a3148129a121f58d2ba37f6c649450100000000000000000000000000000000

a try while still using the domain above.

neithanmo commented 1 month ago

I would give this payload:

0000000001040025a6b5c2c50a2abbff700b87c8dfe29bda77523b0000000000000003000000000000e41800000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000003020000000101021625a6b5c2c50a2abbff700b87c8dfe29bda77523b16dbd1c48f77bf2f9506a3d79117fc1c7eda3b89100f577261707065642d426974636f696e0f777261707065642d626974636f696e05000000000015e3070216982f3ec112a5f5928a5c96a914bd733793b896a51f61726b6164696b6f2d7661756c74732d6f7065726174696f6e732d76312d320a6f70656e2d7661756c740000000b0616982f3ec112a5f5928a5c96a914bd733793b896a51b61726b6164696b6f2d7661756c74732d746f6b656e732d76312d310616982f3ec112a5f5928a5c96a914bd733793b896a51961726b6164696b6f2d7661756c74732d646174612d76312d310616982f3ec112a5f5928a5c96a914bd733793b896a51b61726b6164696b6f2d7661756c74732d736f727465642d76312d310616982f3ec112a5f5928a5c96a914bd733793b896a52061726b6164696b6f2d7661756c74732d706f6f6c2d6163746976652d76312d310616982f3ec112a5f5928a5c96a914bd733793b896a51c61726b6164696b6f2d7661756c74732d68656c706572732d76312d310616982f3ec112a5f5928a5c96a914bd733793b896a51461726b6164696b6f2d6f7261636c652d76322d330616dbd1c48f77bf2f9506a3d79117fc1c7eda3b89100f577261707065642d426974636f696e010000000000000000000000000015e307010000000000000000000000001ddca7400a0516c9a03720732a3148129a121f58d2ba37f6c649450100000000000000000000000000000000

a try while still using the domain above.

Ok this payload triggers an error, i am assuming it does not contains a domain. i replaced the previous payload with this one, and logs seem to indicate that there is more data than expected, this is considered an error

|SP| 500036f8 5000aee8 (30700) : **Domain parsed
|SP| 500036f8 5000aec0 (30660) : Value::from_bytes
|SP| 500036f8 5000aee8 (30700) : **Remainder data not empty

So, after parsing the payload as a clarity value, the parser found unexpected data in the input buffer. Can you check if more data is being appended to the serialized payload and sent to the device?

neithanmo commented 1 month ago

Is the payload not a tuple?

0000000001040025a6b5c2c50a2abbff700b87c8dfe29bda77523b0000000000000003000000000000e41800000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000003020000000101021625a6b5c2c50a2abbff700b87c8dfe29bda77523b16dbd1c48f77bf2f9506a3d79117fc1c7eda3b89100f577261707065642d426974636f696e0f777261707065642d626974636f696e05000000000015e3070216982f3ec112a5f5928a5c96a914bd733793b896a51f61726b6164696b6f2d7661756c74732d6f7065726174696f6e732d76312d320a6f70656e2d7661756c740000000b0616982f3ec112a5f5928a5c96a914bd733793b896a51b61726b6164696b6f2d7661756c74732d746f6b656e732d76312d310616982f3ec112a5f5928a5c96a914bd733793b896a51961726b6164696b6f2d7661756c74732d646174612d76312d310616982f3ec112a5f5928a5c96a914bd733793b896a51b61726b6164696b6f2d7661756c74732d736f727465642d76312d310616982f3ec112a5f5928a5c96a914bd733793b896a52061726b6164696b6f2d7661756c74732d706f6f6c2d6163746976652d76312d310616982f3ec112a5f5928a5c96a914bd733793b896a51c61726b6164696b6f2d7661756c74732d68656c706572732d76312d310616982f3ec112a5f5928a5c96a914bd733793b896a51461726b6164696b6f2d6f7261636c652d76322d330616dbd1c48f77bf2f9506a3d79117fc1c7eda3b89100f577261707065642d426974636f696e010000000000000000000000000015e307010000000000000000000000001ddca7400a0516c9a03720732a3148129a121f58d2ba37f6c649450100000000000000000000000000000000

it starts with 1, which is the clarity value identifier for uint:

pub enum ValueId {
    Int = 0x00,
    UInt = 0x01,
    Buffer = 0x02,
    BoolTrue = 0x03,
    BoolFalse = 0x04,
    StandardPrincipal = 0x05,
    ContractPrincipal = 0x06,
    ResponseOk = 0x07,
    ResponseErr = 0x08,
    OptionalNone = 0x09,
    OptionalSome = 0x0a,
    List = 0x0b,
    Tuple = 0x0c,
    StringAscii = 0x0d,
    StringUtf8 = 0x0e,
}

that is why the parse reads that and finishes, leaving remaining data in the buffer which triggers an error.

in the sip018 documentation, the following is recommended: Structured Data can be any valid Clarity Value. It is assumed that it will usually take the form of a tuple type, as it allows for multiple fields with readable names to be encoded. The standard does permit it to be another type to remain flexible. Still, developers should consider the informational content of requesting a user to sign a simple type like a principal, int, uint, or bool. A tuple can of course contain any type.

even though our implementation reads any clarity value type, it is better to have a tuple, in the provided payload, we read an integer and return, we do not have a way to know how many other values are left in the buffer, of course, we can try to check if there is more data in the buffer, but it seems more error-prone and hacky

314159265359879 commented 1 month ago

@philiphacks see the above, is the payload type something that changed on your side from v1 to v2?

lilm101 commented 1 month ago

Hi there, im getting the same error trying to launch a memecoin on stx.city i keep getting the following error: Data Invalid Your Ledger device has rejected the payload stating it is invalid

Has there been any sollution for this yet?

philiphacks commented 1 month ago

@314159265359879 I doubt it - I don't know what we would have changed since the implementation works with SW wallets; not sure I understand the question :)

lgalabru commented 3 weeks ago

@pete-watters thank you for coordinating the investigation with Zondax's team! any update?

pete-watters commented 3 weeks ago

Hi @lgalabru,

I don't have a further update yet. When I spoke to the Zondax team they ran the original data through their simulator and it worked. They then came back to me with these Q's about the data provided:

The error message we see in Leather Your ledger device has rejected the payload stating it is invalid" is a generic one we show in the UI when ledger rejects the transaction. It could reject the transaction for many reasons, including data issues and a possible bug.

In @314159265359879 's original report he says

It wasn't an issue prior to this vault v2 merge on Arkadiko's side https://github.com/arkadiko-dao/arkadiko/pull/557

I'm unsure if it's to do with the data being passed as the Zondax team mention

Do you have a particular use case you are seeing this error for? Is it with Arkadiko or doing something else? If it's different and you can give me some reproduction steps I am happy to take a look and gather some more information on it to try and solve this

314159265359879 commented 1 week ago

With stacks app version 0.24.2 I nolonger get the "your ledger device has rejected the payload stateing it is invalid" but I still can't proceed. Now the Ledger device screen goes black. It looks like the whole thing turns off.

I see this when opening a vault on arkadiko.finance trying to sign a simple swap transaction on bitflow.finance or alexlab.co

I tested this with two ledger devices, I thought maybe the device was faulty. It is the same on both.

As soon as I click "confirm" in the pop-over it just remains on loading and the device turns off:

image

pete-watters commented 1 week ago

With stacks app version 0.24.2 I nolonger get the "your ledger device has rejected the payload stateing it is invalid" but I still can't proceed. Now the Ledger device screen goes black. It looks like the whole thing turns off.

I see this when opening a vault on arkadiko.finance trying to sign a simple swap transaction on bitflow.finance or alexlab.co

I tested this with two ledger devices, I thought maybe the device was faulty. It is the same on both.

As soon as I click "confirm" in the pop-over it just remains on loading and the device turns off:

image

Thanks for testing this @314159265359879 . This seems like a different issue but as you said it also happens with Xverse it could be a new problem with the Zondax app.

neithanmo commented 3 days ago

Has anyone investigated this issue?

We need to adhere to the specification's recommendations, which involve using tuples for passing arguments. This method is safer than reading arguments one by one until the buffer is exhausted, as it prevents the risk of reading invalid bytes when the number of arguments is unknown. Unless this is fixed in the wallets currently in use, this error will continue to occur.

In my previous message, I analyzed the passed payload and noticed that the arguments are not being passed as a Clarity tuple:

tuple(arg1, arg2, arg3)

Instead, they are concatenated directly as:

arg1 || arg2 || arg3

This is error-prone because our parser expects arguments in tuple format, as recommended by the specification. We recommend updating the wallets or clients to pass arguments using tuples to prevent this issue from recurring.

kyranjamie commented 2 days ago

Trying to make sense of this thread. This issue discusses problems with both stx_signMessage and signTransaction specifically with contract calls, which makes it difficult to follow.

@philiphacks mentions in https://github.com/leather-io/extension/issues/5115#issuecomment-2088717168 that he found a similar error when signing a message. Are we certain these are related, or is it just a similar error?

@neithanmo In https://github.com/leather-io/extension/issues/5115#issuecomment-2251893456 you share a Stacks transaction payload, and then cite the SIP-018 structured message SIP. If I'm not mistaken, this SIP has no relevance to transaction signing?

314159265359879 commented 1 day ago

Thanks @kyranjamie, I did some more investigating also checking differences with different Stacks app versions. It looks like they are unrelated. Just throwing the same general error.

Way forward: With this issue lets focus only on resolving the issue with Arkadiko Vaults. For which we have reproduction steps in the first post.

Sidenotes I have moved the StackingDAO issue mentioned here https://github.com/leather-io/extension/issues/5115#issuecomment-2088717168 to https://github.com/leather-io/extension/issues/5864 that looks to have been resolved. Although the latest Stacks App update has a bug that is causing other issues now a solution for that https://github.com/Zondax/ledger-stacks/issues/171 is in the works.

This comment relates to this issue https://github.com/leather-io/extension/issues/4683 and is partially resolved on the live version and completely resolved with version 0.24.4+ (not yet live).