Zondax / ledger-stacks

Apache License 2.0
17 stars 7 forks source link

Stacks Transactions being rejected - Arkadiko, ALEX + Gamma #159

Open pete-watters opened 6 days ago

pete-watters commented 6 days ago

We are having some issues signing certain transactions in Leather Wallet. https://github.com/leather-io/extension/issues/5115 . It seems like this is an issue with the ledger-stacks app as it is also causing problems when we try with Xverse.

Is this an issue that has been fixed and will be released soon? https://github.com/Zondax/ledger-stacks/issues/157 ?

The problem is happening on Ledger nano S plus (firmware v1.1.1), And Ledger Nano X, with the latest firmware

The message we get back is:

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

The problem can be produced using Arkadiko finance:

Users are also having this same issue when visiting Gamma Stacks NFT marketplace and trying to use Ledger. The Bitcoin ledger app is working but the Stacks app has this same error.

:link: zboto Link

neithanmo commented 6 days ago

Thanks, @pete-watters there are many changes in #158 and #156 might solve those issues. it is possible to test them in a real device using the app/pkg/nanos@xx.sh installers

neithanmo commented 6 days ago

The problem is happening on Ledger nano S plus (firmware v1.1.1), And Ledger Nano X, with the latest firmware

Do you know if ledger Nanos also failed? Would it be possible to share the data being sent to the device? it would facilitate finding the issue

pete-watters commented 5 days ago

Thanks @neithanmo . I will test with a Nano S to see if that also fails. I can gather the data being sent also to help find the problem.

pete-watters commented 5 days ago

@neithanmo I tested just now and it is also failing using a Nano S. The easiest way to replicate it is to try and generate a signature on Stacking DAO.

  1. Go to https://app.stackingdao.com/signer?pool=SP4SZE494VC2YC5JYG7AYFQ44F5Q4PYV7DVMDPBG.stacking-pool-v1
  2. Authenticate with Leather Wallet
  3. Click on either of the "Save Commit Signature" buttons and sign the message

The payload we are sending is:

Screenshot 2024-07-02 at 17 17 09

neithanmo commented 4 days ago

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

neithanmo commented 4 days ago

i do not see any save signature button

image

pete-watters commented 4 days ago

i do not see any save signature button

image

Thanks @neithanmo .

This issue isn't actually a problem in Leather Wallet itself. It occurs when trying to authorise transactions via Leather.

So if you:

On Stacking Dao you should see a screen like this where you can replicate the error:

Screenshot 2024-07-03 at 06 57 15

In the meantime I will try and get the serialised data in case it helps. Thanks!

neithanmo commented 1 day ago

Thanks @pete-watters. Is there any update on the structured_message being sent to the ledger? maybe the serialized data? this would help a lot. but looking at the JSON payload you shared, the app checks the domain, and errors, if there are more/less than 3 fields(name, version, and chain_id), represented as a clarity tuple but it does not verify their values, later it recursively reads the rest of the message checking only that the clarity values are correct:

    pub fn verify(input: &'a [u8]) -> Result<(), ParserError> {
        let rem = Self::parse_prefix(input)?;
        // parse domain
        let (msg, _) = Domain::from_bytes(rem)?;

        // validate msg, but here we can limit the complexity
        // of the msg we can parse as the device has limited resources.
        //let's evaluate if a recursion limit of MAX_DEPTH is good for
        // general purposed structured data
        Self::validate(msg)?;

        Ok(())
    }

we have a recursion limit of MAX_DEPTH, used in case of nested values in the message. However looking at the payload image you share, I do not see complex data structures there that might trigger this limit