Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG: transferred NFT did not transferred to recipient, but got escrowed #517

Open
taitruong opened this issue Mar 16, 2023 · 28 comments
Open
Labels
ics-721 Possible protocol vulnerability

Comments

@taitruong
Copy link
Contributor

taitruong commented Mar 16, 2023

Found 2 examples:

  1. transfer from Stargaze to Juno
    Full flow: Irisnet -> Stargaze -> Juno -> Uptick -> Juno (back transferred) -> Stargaze (back transferred) -> Irisnet (backtransferred) -> Stargaze (retransferred) -> Juno (retransferred, ack error)
  2. transfer back from Stargaze to Irisnet
    Full flow: Irisnet -> Stargaze -> Irisnet (backtransferred, ack error)

In both cases ack failed. As a result NFT is not returned/unlocked on source chain (Stargaze):

  1. Stargaze -> Juno
    a) transfer: https://gon.ping.pub/stargaze/tx/36E5AC4BF6EF7C7522031F9B27DA910BD2C44C14AC082E514596BDBD963E1379
    b) ack: https://gon.ping.pub/juno/tx/499CA03C3E91CB958E76431468BC3772513AF12CFE9273C5268A85DEA2338822
    error:
  • raw log: {"key":"packet_ack","value":"{"error":"codespace: wasm, code: 5"}
  • message: code 5, rpc error: code = NotFound desc = packet acknowledgement hash not found: key not found
  1. Stargaze -> IRISnet (back transfer)
    a) transfer: https://gon.ping.pub/stargaze/tx/0364AD07BBCF638122F0F35D376F9B27BA91A5E01B37C436CA015D20B0EEC919
    transferred succesfully

b) ack: https://gon.ping.pub/iris/tx/F3964D610E30F3CD74353A4BE9191CBEBECCF8D2378D9BED42A89FCE80904408
ack error:

  • raw log: {"key":"packet_ack","value":"{"error":"ABCI code: 1: error handling packet: see events for details"}
  • decoding bech32 failed: invalid separator index 11 | decoding bech32 failed: invalid separator index 11
@taitruong
Copy link
Contributor Author

More details on case 1:
i) token was successfully transfer before, starting from IRISnet -> Stargaze -> Juno -> Uptick
ii) succesfully send back from Uptick -> Juno -> Stargaze -> Irisnet
iii) resend Irisnet -> Stargaze -> Juno
Last transfer is where it got stucked on Juno. There is another bug #501 outlining when token got received second time. There it is not clear, what '2nd time' means. 2nd transfer or second packet receive?

@taitruong
Copy link
Contributor Author

Also bug #497 has another example. maybe this is all related with each other.

@taitruong
Copy link
Contributor Author

Found 2 examples:

  1. transfer from Stargaze to Juno
  2. transfer back from Stargaze to Irisnet

In both cases ack failed. As a result NFT is not returned/unlocked on source chain (Stargaze):

  1. Stargaze -> Juno
    a) transfer: https://gon.ping.pub/stargaze/tx/36E5AC4BF6EF7C7522031F9B27DA910BD2C44C14AC082E514596BDBD963E1379
    b) ack: https://gon.ping.pub/juno/tx/499CA03C3E91CB958E76431468BC3772513AF12CFE9273C5268A85DEA2338822
    error:
  • raw log: {"key":"packet_ack","value":"{"error":"codespace: wasm, code: 5"}
  • message: code 5, rpc error: code = NotFound desc = packet acknowledgement hash not found: key not found
  1. Stargaze -> IRISnet (back transfer)
    a) transfer: https://gon.ping.pub/stargaze/tx/0364AD07BBCF638122F0F35D376F9B27BA91A5E01B37C436CA015D20B0EEC919
    transferred succesfully

b) ack: https://gon.ping.pub/iris/tx/F3964D610E30F3CD74353A4BE9191CBEBECCF8D2378D9BED42A89FCE80904408 ack error:

  • raw log: {"key":"packet_ack","value":"{"error":"ABCI code: 1: error handling packet: see events for details"}
  • decoding bech32 failed: invalid separator index 11 | decoding bech32 failed: invalid separator index 11

On case 2: as noted by sender here: https://discord.com/channels/669268347736686612/1074987031270268958/1085843405940195369 (Cosmos Network, gon-testnet channel)
Token was before alsos send from Irisnet -> SG, then it failed on sending back from SG -> Irisnet.

@taitruong taitruong changed the title BUG: transferred NFT did not transferred to recipient, but get escrowed BUG: transferred NFT did not transferred to recipient, but got escrowed Mar 16, 2023
@gjermundgaraba
Copy link
Contributor

I am pretty sure #497 is related here

@taitruong
Copy link
Contributor Author

taitruong commented Mar 16, 2023

@gjermundgaraba still got console log from self relay from example 1 above: https://gon.ping.pub/stargaze/tx/36E5AC4BF6EF7C7522031F9B27DA910BD2C44C14AC082E514596BDBD963E1379

{
"level": "info",
"ts": 1678927780.813735,
"caller": "relayer/naive-strategy.go:569",
"msg": "Failed to relay packet from sequence",
"src_chain_id": "uni-6",
"src_channel_id": "channel-120",
"src_port_id": "wasm.juno1stv6sk0mvku34fj2mqrlyru6683866n306mfv52tlugtl322zmks26kg7a",
"dst_chain_id": "elgafar-1",
"dst_channel_id": "channel-230",
"dst_port_id": "wasm.stars1ve46fjrhcrum94c7d8yc2wsdz8cpuw73503e8qn9r44spr6dw0lsvmvtqh",
"channel_order": "ORDER_UNORDERED",
"error": "packet commitment not found"
},
{
"level": "info",
"ts": 1678927781.8039687,
"caller": "relayer/log-chain.go:31",
"msg": "Failed sending transaction",
"chain_id": "elgafar-1",
"msg-0": {},
"msg-0Error": "Unregistered interface crypto.isPublicKey_Sum",
"msg-1": {
    "msg_json": "{\"packet\":{\"sequence\":\"228\",\"source_port\":\"wasm.stars1ve46fjrhcrum94c7d8yc2wsdz8cpuw73503e8qn9r44spr6dw0lsvmvtqh\",\"source_channel\":\"channel-230\",\"destination_port\":\"wasm.juno1stv6sk0mvku34fj2mqrlyru6683866n306mfv52tlugtl322zmks26kg7a\",\"destination_channel\":\"channel-120\",\"data\":\"eyJjbGFzc0lkIjoid2FzbS5zdGFyczF2ZTQ2ZmpyaGNydW05NGM3ZDh5YzJ3c2R6OGNwdXc3MzUwM2U4cW45cjQ0c3ByNmR3MGxzdm12dHFoL2NoYW5uZWwtMjA3L2dvbkluZGl2UmFjZTEiLCJjbGFzc1VyaSI6Imh0dHBzOi8vZ2l0aHViLmNvbS9nYW1lLW9mLW5mdHMvZ29uLXRlc3RuZXRzI3JvdW5kLTMtY29tcGV0aXRpdmUtaW5kaXZpZHVhbC1yYWNlIiwiY2xhc3NEYXRhIjoiZXlKa1pYTmpjbWx3ZEdsdmJpSTZJbE5qYUdWdFlTQm1iM0lnUTI5c2JHVmpkR2x2Ym5NZ2IyWWdSMjlPSUZOMFlXZGxJRE1nUVdseVpISnZjQ0lzSW1seWFYTnRiMlE2WTNKbFlYUnZjaUk2ZXlKMllXeDFaU0k2SW1aallqTmlaR0pqT0RJNU9HRTROekF4T0dRd016azRPV1JpTnprelpEZzFaakE1TXpJMk9XWWlmU3dpYVhKcGMyMXZaRHBrWlhOamNtbHdkR2x2YmlJNmV5SjJZV3gxWlNJNklrbHVJSFJvYVhNZ2NtOTFibVFzSUhCaGNuUnBZMmx3WVc1MGN5QjNhV3hzSUhKbFkyVnBkbVVnVGtaVUlHRnBjbVJ5YjNCeklHOXVJRWxTU1ZOdVpYUXVJRVZoWTJnZ1RrWlVJR052Ym5SaGFXNXpJRlJoYzJzZ1JHRjBZU0JwYmlCcGRITWdkRzlyWlc0Z1pHRjBZU3dnZDJocFkyZ2dhVzVqYkhWa1pYTWdZU0JtYkc5M0lHWnBaV3hrSUhkcGRHZ2dZU0JqYjNKeVpYTndiMjVrYVc1bklHWnNiM2N0YVdRZ2RtRnNkV1VnZEdoaGRDQmpZVzRnWW1VZ2RYTmxaQ0IwYnlCeGRXVnllU0IwYUdVZ1lXTjBkV0ZzSUdac2IzY2dhVzRnWm14dmR5QjBZV0pzWlM0Z1VHRnlkR2xqYVhCaGJuUnpJR05oYmlCMGFHVnVJSEJsY21admNtMGdTVzUwWlhKamFHRnBiaUJPUmxRZ1ZISmhibk5tWlhJZ2RYTnBibWNnZEdobElITndaV05wWm1sbFpDQm1iRzkzY3k0Z1FXWjBaWElnWTI5dGNHeGxkR2x1WnlCMGFHVWdhVzUwWlhKamFHRnBiaUJPUmxRZ2RISmhibk5tWlhJc0lHVmhZMmdnY0dGeWRHbGphWEJoYm5RZ2JYVnpkQ0IwY21GdWMyWmxjaUIwYUdGMElFNUdWQ0IwYnlCMGFHVWdiR0Z6ZEY5dmQyNWxjaUIwYnlCamIyMXdiR1YwWlNCMGFHVWdjbUZqWlM0aWZTd2lhWEpwYzIxdlpEcHRhVzUwWDNKbGMzUnlhV04wWldRaU9uc2lkbUZzZFdVaU9uUnlkV1Y5TENKcGNtbHpiVzlrT201aGJXVWlPbnNpZG1Gc2RXVWlPaUpuYjI1SmJtUnBkbEpoWTJVeEluMHNJbWx5YVhOdGIyUTZjMk5vWlcxaElqcDdJblpoYkhWbElqb2llMXdpZEdsMGJHVmNJanBjSWtWU1F5MDNNakVnUTI5c2JHVmpkR2x2Ymx3aUxGd2laR1Z6WTNKcGNIUnBiMjVjSWpwY0lsTmphR1Z0WVNCbWIzSWdSVkpETFRjeU1TQkRiMnhzWldOMGFXOXVjMXdpTEZ3aWRIbHdaVndpT2x3aWIySnFaV04wWENJc1hDSnlaWEYxYVhKbFpGd2lPbHRjSW01aGJXVmNJaXhjSW5ONWJXSnZiRndpTEZ3aVpHVnpZM0pwY0hScGIyNWNJbDBzWENKd2NtOXdaWEowYVdWelhDSTZlMXdpYm1GdFpWd2lPbnRjSW5SNWNHVmNJanBjSW5OMGNtbHVaMXdpTEZ3aVpHVnpZM0pwY0hScGIyNWNJanBjSWxSb1pTQnVZVzFsSUc5bUlIUm9aU0JqYjJ4c1pXTjBhVzl1WENKOUxGd2ljM2x0WW05c1hDSTZlMXdpZEhsd1pWd2lPbHdpYzNSeWFXNW5YQ0lzWENKa1pYTmpjbWx3ZEdsdmJsd2lPbHdpVkdobElITjViV0p2YkNCdlppQjBhR1VnWTI5c2JHVmpkR2x2Ymx3aWZTeGNJbVJsYzJOeWFYQjBhVzl1WENJNmUxd2lkSGx3WlZ3aU9sd2ljM1J5YVc1blhDSXNYQ0prWlhOamNtbHdkR2x2Ymx3aU9sd2lRU0JrWlhOamNtbHdkR2x2YmlCdlppQjBhR1VnWTI5c2JHVmpkR2x2Ymx3aWZTeGNJbWx0WVdkbFhDSTZlMXdpZEhsd1pWd2lPbHdpYzNSeWFXNW5YQ0lzWENKbWIzSnRZWFJjSWpwY0luVnlhVndpTEZ3aVpHVnpZM0pwY0hScGIyNWNJanBjSWtFZ1ZWSkpJRzltSUdGdUlHbHRZV2RsSUhKbGNISmxjMlZ1ZEdsdVp5QjBhR1VnWTI5c2JHVmpkR2x2Ymx3aWZYMTlJbjBzSW1seWFYTnRiMlE2YzNsdFltOXNJanA3SW5aaGJIVmxJam9pWjJseU1TSjlMQ0pwY21semJXOWtPblZ3WkdGMFpWOXlaWE4wY21samRHVmtJanA3SW5aaGJIVmxJanAwY25WbGZTd2lhWEpwYzIxdlpEcDFjbWxmYUdGemFDSTZleUoyWVd4MVpTSTZJakF6WW1WaE5qazJZekpqWTJObE56SXpaV1JoTUdVek5EQXhZbVUzTTJabFpUWXpaR1l4Wm1Wak5qWTRNVFZqWm1Vek1URTBNVFUyWlRSbFpUZGxObVlpZlN3aWNISnZjR1Z5ZEdsbGN5STZleUprWlhOamNtbHdkR2x2YmlJNmV5SmtaWE5qY21sd2RHbHZiaUk2SW5Sb1pTQm1hWEp6ZENCeWIzVnVaQ0J2WmlCaGNtbGtjbTl3SUdsdUlISnZkVzVrSURNZ2IyWWdSMjlPSWl3aWRIbHdaU0k2SW5OMGNtbHVaeUo5TENKcGJXRm5aU0k2ZXlKa1pYTmpjbWx3ZEdsdmJpSTZJbWgwZEhCek9pOHZaMmwwYUhWaUxtTnZiUzluWVcxbExXOW1MVzVtZEhNdloyOXVMWFJsYzNSdVpYUnpMMkpzYjJJdmJXRnBiaTloYzNObGRDOW5hWEl4TG5CdVp5SXNJbVp2Y20xaGRDSTZJblZ5YVNJc0luUjVjR1VpT2lKemRISnBibWNpZlN3aWJtRnRaU0k2ZXlKa1pYTmpjbWx3ZEdsdmJpSTZJbWR2YmtsdVpHbDJVbTkxYm1ReElpd2lkSGx3WlNJNkluTjBjbWx1WnlKOUxDSnplVzFpYjJ3aU9uc2laR1Z6WTNKcGNIUnBiMjRpT2lKbmFYSXhJaXdpZEhsd1pTSTZJbk4wY21sdVp5SjlmU3dpY21WeGRXbHlaV1FpT2xzaWJtRnRaU0lzSW5ONWJXSnZiQ0lzSW1SbGMyTnlhWEIwYVc5dUlsMHNJblJwZEd4bElqb2lSMjlPSUVsdVpHbDJhV1IxWVd3Z1VtRmpaU0JTYjNWdVpDQXhJaXdpZEhsd1pTSTZJbTlpYW1WamRDSjkiLCJ0b2tlbklkcyI6WyJnaXIxL3RhaXRydW9uZyJdLCJ0b2tlblVyaXMiOlsiaHR0cHM6Ly9naXRodWIuY29tL3RhaXRydW9uZyJdLCJ0b2tlbkRhdGEiOlsiZXlKbWJHOTNJam9pWmpBeElpd2lhWEpwYzIxdlpEcHVZVzFsSWpwN0luWmhiSFZsSWpvaVoybHlNUzkwWVdsMGNuVnZibWNpZlN3aWFYSnBjMjF2WkRwMWNtbGZhR0Z6YUNJNmV5SjJZV3gxWlNJNkltSXhNbUl3TXpCaU1qWmtZekZpTXpNeE9UQmlPV0prTm1GbU5qazRaalZtTjJZd1ltUTFZbVF3TlRGaE5EY3pabVUwTUdVd1ptWmlZamxrWmpNMU1EQWlmU3dpYkdGemRGOXZkMjVsY2lJNkltbGhZVEUwT0RoM2QzSXlNelYyYTJFM2FqY3lNbWg2WVdOd2F6QndiSGgzTXpOcmMzRjVibVYxZWlJc0luTjBZWEowWDJobGFXZG9kQ0k2SWpRM016QXdNQ0lzSW5SNWNHVWlPaUpwYm1ScGRtbGtkV0ZzSUhKdmRXNWtJREVpZlE9PSJdLCJzZW5kZXIiOiJzdGFyczE4M2U3Y2N3c25uZ2oycThsZnhubWVrdW5zcG5meHM2cThuenFjZiIsInJlY2VpdmVyIjoianVubzE4M2U3Y2N3c25uZ2oycThsZnhubWVrdW5zcG5meHM2cTlha3g1eSIsIm1lbW8iOm51bGx9\",\"timeout_height\":{},\"timeout_timestamp\":\"1678926025319484470\"},\"proof_unreceived\":\"CroICrcICnZyZWNlaXB0cy9wb3J0cy93YXNtLmp1bm8xc3R2NnNrMG12a3UzNGZqMm1xcmx5cnU2NjgzODY2bjMwNm1mdjUydGx1Z3RsMzIyem1rczI2a2c3YS9jaGFubmVscy9jaGFubmVsLTEyMC9zZXF1ZW5jZXMvMjI4EgEBGg0IARgBIAEqBQACoqhDIi0IARIGAgT4qEMgGiEgZkaiHVz13mHJ1Lu3HF9t4jcAjANTNGo5z3FnZupFKI8iKwgBEicEBvioQyA+55F8uz5ybMRZPoKtnyF3wtffDiKc6ivS4LXvvK7p5yAiLQgBEgYGDpCrQyAaISCoLuZ1q+L9kTukSpk/6Zm/e7HsmI5QHqePqSeo/WvhKyIrCAESJwgakKtDIP2Gyi3xwVRxeEQGpRdlXf3JYnuq7aHOBFP96d9pJVC4ICItCAESBgomkKtDIBohIP7vL9w7f++vWg/nsue4r4P1Yg8wCLxYT7QVTE32kyylIisIARInDFSQq0MgCYMDX39Cx2jESYbcyyNQgiAAj2jqO7VG2E0AJevEMW8gIi4IARIHDqIBkKtDIBohIP+53/utnTsTCAame/vTTxwY3wG4gQf8riZSuyFe2g52IiwIARIoEIICkKtDIG3WaHqWXEbh9CClu5fLMzdVoNirB6zBcvAgVmBoCoE6ICIsCAESKBKEA5CrQyCimEzfnnGvEp2anPZk7UwzoMh97htzJG0LAioc9Oe8lCAiLggBEgcU4Aawq0MgGiEgLfSsLt0h3leTHzdVbiH+zhRAONpzpBCSsiWsfM3DyFsiLggBEgcW8Amwq0MgGiEgK/5BSqQtEjpQL/Vh15AqCeXbPSFGoDrUhdIN4GnL30siLggBEgcY5A+wq0MgGiEgFyfHoHirdmLCRLOtzjuthWYkVTyHKhoZ4jDDpfAcNpUiLggBEgca5hiwq0MgGiEg4O5OBmWxTbSeh6jYd0o8ts1nHfAKppDfb/hUDWpC68AiLggBEgcc7C6wq0MgGiEg5WxfART9FhURix9Aly1iRA2liEFH0gnc5yqwBL6Nc5QiLggBEgcelGKwq0MgGiEgIDJrjzGNpB5678oj60hEzV0azNBgB/sUKKjQxkU96HMiLwgBEgggxK0BsKtDIBohIDRHQtS8CFijxaLrFiLwhfHR6gtnrHPlh2h4flxablUpIi0IARIpIoTSArCrQyCUbnsc/Y7ODrjLkXYBCZadfzYtUqPd+nDxXxbHIfbzCCAiLQgBEikk2qkF2K1DIAR1AMT5Kv9aCu0/LjnplQf6R1LnsnVcnvAs2vR/wnGjICItCAESKSbCsw/YrUMghSnSAWlZC8h/ACqoeW39a4iX1KKvKvlZDhfUtUV8dkMgIi0IARIpKOKHFditQyAyPzMh2/GIYEDhiiFn2d6SA/OowlEuMGspefiA5BbtfSAK/gEK+wEKA2liYxIgnpeo1ym06WrXReUnLCr/nPC4PmZGDX3mkIUpYPuCSoEaCQgBGAEgASoBACInCAESAQEaIMUFwP1Isc8rZWGfErMUThnGC05rYlJe6Ta+k9BBYj6/IiUIARIhAWMRZNhyN89WOtl0zOZ7s0KK6zy6OKOhIdzX/A5qBCl/IicIARIBARogtAQg/xP8BJPD93t7HCBfIVqjqM3TxWRYhEoI/fVqAW0iJQgBEiEBRuY4N2rmaiRI56l5u+eVMeLi13nCCwF/NrpQlGJkwz4iJwgBEgEBGiA/fenklLIEnN26fjFLQouAWNb2lllYihsKdmZ1Fi11Lg==\",\"proof_height\":{\"revision_number\":\"6\",\"revision_height\":\"551793\"},\"next_sequence_recv\":\"228\",\"signer\":\"stars1g0krhq56pmxmaz8lnj8h6jf55kcsq7x0lw5ywc\"}"
},
"error": "packet messages are redundant",
"errorVerbose": "packet messages are redundant\ncosmossdk.io/errors.Wrap\n\t/home/ttruong/go/pkg/mod/cosmossdk.io/[email protected]/errors.go:187\ncosmossdk.io/errors.ABCIError\n\t/home/ttruong/go/pkg/mod/cosmossdk.io/[email protected]/errors.go:75\ngithub.com/strangelove-ventures/lens/client.broadcastTx\n\t/home/ttruong/go/pkg/mod/github.com/strangelove-ventures/[email protected]/client/broadcast.go:86\ngithub.com/strangelove-ventures/lens/client.(*ChainClient).BroadcastTx\n\t/home/ttruong/go/pkg/mod/github.com/strangelove-ventures/[email protected]/client/broadcast.go:36\ngithub.com/cosmos/relayer/v2/relayer/chains/cosmos.(*CosmosProvider).SendMessages.func1\n\t/home/ttruong/go/pkg/mod/github.com/cosmos/relayer/[email protected]/relayer/chains/cosmos/tx.go:161\ngithub.com/avast/retry-go/v4.Do\n\t/home/ttruong/go/pkg/mod/github.com/avast/retry-go/[email protected]/retry.go:128\ngithub.com/cosmos/relayer/v2/relayer/chains/cosmos.(*CosmosProvider).SendMessages\n\t/home/ttruong/go/pkg/mod/github.com/cosmos/relayer/[email protected]/relayer/chains/cosmos/tx.go:85\ngithub.com/cosmos/relayer/v2/relayer.(*RelayMsgs).send\n\t/home/ttruong/go/pkg/mod/github.com/cosmos/relayer/[email protected]/relayer/relayMsgs.go:222\nruntime.goexit\n\t/usr/local/go/src/runtime/asm_amd64.s:1594"
}

@taitruong
Copy link
Contributor Author

image

@taitruong
Copy link
Contributor Author

I am able to reproduce the bug, I tried it a couple of times, it always fails. On retransfer from Stargaze to Juno, it got minted on Juno again, but it is escrowed and not transferred to recipient!

Transfer flow:

visit:
irisnet -> stargaze -> juno
back:
stargaze -> juno -> irisnet
re-transfer:
irisnet -> stargaze -> juno (here it fails)

So on back transfer it fails:
a) transfer: https://gon.ping.pub/stargaze/tx/9E113F219DB9B357FFA093CB972C117771C6DD2F85A1E558CEB3DE17C9FC915B

b) ack: https://gon.ping.pub/juno/tx/B6C649A2093E3E5E00817108448CC230B661C35DB67D430D3BCBD062F7C7AE68
code 5, rpc error: code = NotFound desc = packet acknowledgement hash not found: key not found

@taitruong
Copy link
Contributor Author

For reproducing the bug, I simply created a new collection. So bug occurs for any NFT and collection.

@beal2912
Copy link
Contributor

Hello,

I have replicated the problem with my second airdrop : gir2/beal2912 on Flow F31

What I did was

OK : i - 2 - s + gon : 57D4D11C47CC5FF626459C51D38065C675E48D4882E9733BC0907A7C00238B84
OK : s - 2 - J + gon : 2D06F9E97C0142A8FEE52C02B639608A37F70944061FFED27845E58D25B50AA9

then roll back :
OK : j - 2 - s + gon : 8DAC483CED9C5D34BA1F535E2431C89493BBE9DA6BC38CAD21D571537F025600
OK : s - 2 - i + gon : 6B6CB5E4C4140F420B65263F275F18EED051F34C9C57EF09253954D100BD184F

then restart :

KO : i - 2 - s + gon : 161C06F168CD1FFEC3C7414DF7DB28F16F01D6BBE6586ACF8A0A0B7D0F542948
the tx is successful but with "packet acknowledgement hash not found"

my assumption is that I used the path a 2 times with self relay with GON, and the 2nd time there is a mix up between acknowledgement packet but the tx is successul so the nft stay in the escrow address,

@taitruong
Copy link
Contributor Author

I also tried with other chains: J(uno), U(ptick), O(mniflix), and I(risnet)

  1. visits
    J -> U -> O -> I
  2. back visit:
    I -> O -> U -> J
  3. Revisit
    J -> U -> O -> I

But couldn' reproduce it here. Only when Stargaze and Juno is used. This might be related to this spam attack on channel between Stargaze and Juno: #479

@taitruong
Copy link
Contributor Author

I'm wondering whether it has something to do with packet sequences. I dunno how numbers are generated. But once I had a packet sequence 14005 (transfer from Juno to Stargaze) and later a lower sequence number of 282. As noted in #479 malicious attack transferred ~500k NFTs.

@giansalex
Copy link
Contributor

giansalex commented Mar 16, 2023

only occurs with Juno/Stargaze because they have a different implementation than nft-transfer module,
cw-ics721 needs to receive IBC ACK to burn token, but it does not arrive in time (due to spam). is as indicated in issue #501

@giansalex
Copy link
Contributor

then restart :
KO : i - 2 - s + gon : 161C06F168CD1FFEC3C7414DF7DB28F16F01D6BBE6586ACF8A0A0B7D0F542948
the tx is successful but with "packet acknowledgement hash not found"

Your last tx got timeout: 5D9FD20013EF5A97CD1960F6C6A1F8996A4C49FC6B95260F50D2A12E53CEDE76

msg: packet acknowledgement hash not found, it's not part of the transaction result, it's from the explorer, is causing confusion

@taitruong
Copy link
Contributor Author

taitruong commented Mar 16, 2023

I got a different TX.
last tx @beal2912 mentions: https://gon.ping.pub/iris/tx/161C06F168CD1FFEC3C7414DF7DB28F16F01D6BBE6586ACF8A0A0B7D0F542948

search for ack on stargaze:
clear;starsd q txs --events "recv_packet.packet_sequence=146&recv_packet.packet_src_port=nft-transfer&recv_packet.packet_src_channel=channel-23&recv_packet.packet_dst_port=wasm.stars1ve46fjrhcrum94c7d8yc2wsdz8cpuw73503e8qn9r44spr6dw0lsvmvtqh&recv_packet.packet_dst_channel=channel-208"

led to: https://gon.ping.pub/stargaze/tx/26D361FCE11E66FF2A30DE08E1EE70899549C1A054D936D8FF7B62C2C1C10500

But same error. In any case ICS721 contract should rollback on timeout. @0xekez

@taitruong
Copy link
Contributor Author

taitruong commented Mar 16, 2023

@giansalex why do you think it is a timeout? On dest chain checking this: https://gon.ping.pub/stargaze/tx/26D361FCE11E66FF2A30DE08E1EE70899549C1A054D936D8FF7B62C2C1C10500

tx height : 3885887
timestamp: 2023-03-16T12:08:24Z
timeout height is: 3886875 (later than height)
timeout timespam: 1678969095276252876 (10 min later than timestamp)

@giansalex
Copy link
Contributor

@giansalex why do you think it is a timeout? On dest chain checking this: https://gon.ping.pub/stargaze/tx/26D361FCE11E66FF2A30DE08E1EE70899549C1A054D936D8FF7B62C2C1C10500

tx height : 3885887 timestamp: 2023-03-16T12:08:24Z timeout height is: 3886875 (later than height) timeout timespam: 1678969095276252876 (10 min later than timestamp)

That was the result of using gon-cli, maybe a issue there.

@giansalex
Copy link
Contributor

giansalex commented Mar 16, 2023

@taitruong I tried again and Timeout Tx is accepted, even though ACK was already received. Although this does not affect the token owner (nft-transfer is not called), this is probably solved by adding IBC Ante.

Transfer: 161C06F168CD1FFEC3C7414DF7DB28F16F01D6BBE6586ACF8A0A0B7D0F542948
RCV: 26D361FCE11E66FF2A30DE08E1EE70899549C1A054D936D8FF7B62C2C1C10500
ACK: 88CC2FFAC28489A4BA5E7134392C249322DC795E6DC5FF778B566611AAE43084

Timeout:
5D9FD20013EF5A97CD1960F6C6A1F8996A4C49FC6B95260F50D2A12E53CEDE76
32D7207D342190FBE9B758AD3C93B7769659F56D13B8A26BF5850C3842AEDC88

@taitruong
Copy link
Contributor Author

Summary

Collection originally minted on IRISnet. Transfer through various chains: I(RISnet), S(targaze), J(uno), U(ptick), O(mniflix)

Alright summing things up. Problem is described here: #501:

tl;dr: retransferring (backtrack of an) NFT in ICS721 wasm contract, burns on ACK response ("locked" chain until then). As long as ACK is not send/relayed yet, it keeps locked. Though on other ("active) chain it is active/unlocked. Sending an NFT from "active" to "locked" chain is not possible, since NFT is still there and not yet burned. Once ACK has been send, NFT can be send to "burned" chain.

Problem

token gets transferred throught these chains:

  1. A -> B: NFT minted on chain B and locked on chain A
  2. B -> A: nft is unlocked on chain a, token is not getting burned on chain b, because ack does not arrive
    3: A -> B: token is locked on chain a and transferred to chain b, but can't because it is already there.

For 3) NFT results can be:
a) active on chain A, locked-not-yet-burned on chain B
b) locked-not-yet-unlocked on both chain A and locked-not-yet-burned on chain B

Examples below.

Active on chain A, Locked-Not-Yet-Burned on chain B

Affected chains: IRISnet and Stargaze

For a) active on chain A, locked-not-yet-burned on chain B

  • token was sent I > S and then back to > I
  • result: Now token is active on I, but locked on S

user tried sending from I back to S, but it failed ("error":"codespace: wasm, code: 5") and got reverted/unlocked back to I
multiple attempts afterwards I > S, it all failed:

transfer from I: https://gon.ping.pub/iris/tx/92E4C554A216237B0880DFA3D47A995198A71F279A375FA5FABB2000F7E401BC
ack error from S: https://gon.ping.pub/stargaze/tx/D67326D58BAD39CC169A47989DB4A44294F5B723349A32527EFE304D29BA4E37

transfer from I: https://gon.ping.pub/iris/tx/88BC117968E47E2B82C00D5B76317F3035A3A517DB7F9C00324CB36B6877C0D0
ack error from S: https://gon.ping.pub/stargaze/tx/803DD8471D12B2E9D32CBA0972CB9C43346DC06F7733A2FB5929E6736D342004

transfer from I: https://gon.ping.pub/iris/tx/BEFCFEFAADCECEFE8B48A048F1B3DDF2613EBDC6A43C1D50F1984D4C1D77FF14
ack error from S: https://gon.ping.pub/stargaze/tx/2A930AE1C93857D9B9D1A71718CB3C9C3270316D194961F91965BDA141ACEF58

transfer from I: https://gon.ping.pub/iris/tx/5EC4873825477B6773EEB1061FD57B7AB36EA4E496295E371A5C3D6AC7419263
ack error from S: https://gon.ping.pub/stargaze/tx/ABA9831C16CFEE0BD96B6EF63CF0ECE17F13EF7C46D923A3E682F9E5F38BCD3C

Locked-Not-Yet-Unlocked on both chain A and Locked-Not-Yet-Burned on chain B

Affected chains: Stargaze and Juno

This is prob the worst case:

  1. NFT got send I > S > J
  2. send back J > S > I
  3. resend I > S > J

When it got send back in 2) from J > S, NFT got locked-not-yet-burned on Juno.
In 3) when sending from S > J, it failed - since it is still locked and not burned on Juno, in addition ack fail from J > S got stucked, so on S side NFT is locked-not-yet-unlocked since ack hasn't arrived yet.

transfer from Stargaze:
https://gon.ping.pub/stargaze/tx/36E5AC4BF6EF7C7522031F9B27DA910BD2C44C14AC082E514596BDBD963E1379
ack error from Juno: https://gon.ping.pub/juno/tx/499CA03C3E91CB958E76431468BC3772513AF12CFE9273C5268A85DEA2338822

@taitruong
Copy link
Contributor Author

Due to the Spam between Stargaze and Juno on a specific channel, there are right now ~470k open packets. Even though NFTs are send now through different channels relayers got stucked and are not able to relay all packets.

We know that the root cause is that ACK is not send yet.

@gjermundgaraba, it is possible that gon cli's self relaying feature is "hiding" this problem, because self-relay:
picks a specific packet sequence to relay. at least it relays recv package, but 99% sure ack is NOT relayed.

Why? Once NFT passed either Stargaze or Juno, ACKs gets stucked!

Normally relayers puts new packets in queue. Normal use case is that for congested chains like Stargaze and Juno, all incoming packets are queued. So transferred NFT gets queued, but not transferred (minted or burned) on Stargaze or Juno. User would than immediately see that NFT isn't there yet. With self-relay NFT at least can pass ONE time, next time sending back and self-relay again, it fails immediately because NFT is not yet burned.

@giansalex
Copy link
Contributor

giansalex commented Mar 16, 2023

The problem is that token burning should occur at Transfer time, not at the IBC ACK.

will be reviewed next week #501 (comment)

@taitruong
Copy link
Contributor Author

The problem is that token burning should occur at Transfer time, not at the IBC ACK.

will be reviewed next week #501 (comment)

Not sure whether this solves this, because of this case:
back transfer from A to B, e.g with wrong/incorrect recipient address.

normally on chain B it sends an ACK fail for rollback, but since ACK gets stucked, NFT is neither on chain B nor on chain A. Once ACK fail has arrived on chain B, it got reminted and returned back to sender.

@giansalex
Copy link
Contributor

giansalex commented Mar 16, 2023

Not sure whether this solves this, because of this case: back transfer from A to B, e.g with wrong/incorrect recipient address.

normally on chain B it sends an ACK fail for rollback, but since ACK gets stucked, NFT is neither on chain B nor on chain A. Once ACK fail has arrived on chain B, it got reminted and returned back to sender.

if the NFT is neither on chain A nor B, the user must wait for the relayers. This is a normal case that also occurs when the packet is in Timeout and the relayers are not operational.

in case #501, the remote wasm chain cannot accept the NFT, due to an internal error, unexpected behavior

@ElfenLied2019
Copy link
Contributor

I have tried multiple times from i -- (1) -->s -- (1) -->o -- (1) -->j -- (1) -->i, and each time I lose nft at o -- (1) -->j

https://gon.ping.pub/omniflix/tx/845A21636100E5644A7C50F00D5AEB64D3BD5ACC1E4C87E83785AAC445A79DDC
https://gon.ping.pub/omniflix/tx/EE3F6EE8E874B633C0985C32659953942E137C94A543B6914DA870F177E7F2AB

@taitruong
Copy link
Contributor Author

Not sure whether this solves this, because of this case: back transfer from A to B, e.g with wrong/incorrect recipient address.
normally on chain B it sends an ACK fail for rollback, but since ACK gets stucked, NFT is neither on chain B nor on chain A. Once ACK fail has arrived on chain B, it got reminted and returned back to sender.

if the NFT is neither on chain A nor B, the user must wait for the relayers. This is a normal case that also occurs when the packet is in Timeout and the relayers are not operational.

in case #501, the remote wasm chain cannot accept the NFT, due to an internal error, unexpected behavior

There is no internal error. Receiving chain either sends an ack success or ack fail. Acks are always relayed. Read here: https://medium.com/the-interchain-foundation/everything-you-need-to-know-about-ibc-channels-faqs-part-2-44249a7b7e0b

Do acknowledgements have timeouts?

No, they do not. There needs to be at least one message that can be submitted async. Otherwise it causes a circular problem where if timeouts existed for acknowledgements, then the message proving that the acknowledgment timed out could itself timeout, and so on — causing an endless loop.

@giansalex
Copy link
Contributor

giansalex commented Mar 18, 2023

There is no internal error. Receiving chain either sends an ack success or ack fail. Acks are always relayed. Read here: https://medium.com/the-interchain-foundation/everything-you-need-to-know-about-ibc-channels-faqs-part-2-44249a7b7e0b

I am referring to the error that happens when cw-ics721 executes the cw721 contract on IBC Recv event.

{"error":"codespace: wasm, code: 5"}

This error should not occur if cw-ics721 would faithfully follow the ICS721 spec.

@taramakage taramakage added the ics-721 Possible protocol vulnerability label Mar 27, 2023
@taitruong
Copy link
Contributor Author

taitruong commented Mar 27, 2023

There is no internal error. Receiving chain either sends an ack success or ack fail. Acks are always relayed. Read here: https://medium.com/the-interchain-foundation/everything-you-need-to-know-about-ibc-channels-faqs-part-2-44249a7b7e0b

I am referring to the error that happens when cw-ics721 executes the cw721 contract on IBC Recv event.

{"error":"codespace: wasm, code: 5"}

This error should not occur if cw-ics721 would faithfully follow the ICS721 spec.

There has been a long discussion about this between IRISnet, Stargaze and Juno. The spec shows one possible solution. In this case to burn on transfer while sending back. Another solution is burn on ACK success. Technically both solutions are fine.

From functional/business perspective, the question is: Do you really want to burn your NFT on chain B, while complete flow is not finished yet? As long as chain A is not finished. In the meanwhile for the holder its intermediate state is like this:

  • holder does not hold it anymore, since it is burned on chain B
  • NFT is escrowed on chain A

While in the other case NFT is escrowed on both chains. It might be subtle, but during GoN a lot suffered from this. And most of participants had the feeling their NFT got lost and they couldn't find it on chain B anymore. They saw nothing on chain B has happend, at least some realized it is still escrowed on chain A.

It is just my opinion, but technically both fulfills the same purpose and leads to the same result. But from user's perspective it is not only a technical issue, they get the impression of having their NFT lost.

Users start panicking, don't understand what is going on, thinking about they got ruggeg or whatever. For them it is just gone.

@taitruong
Copy link
Contributor Author

After all, at least we have identified the source of this: packets have not been send, first implementation of self-relay only relayed transfer packet, but not ack packet. Now with self-relay at least all packets for given transfer has been relayed.

So without relay, it lead NFT being:

  • burned on chain B
  • escrowed on chain A

With relaying only transfer packet, it led to same result, but it made it worst when doing a re-transfer, so complete flow is like:

  1. I transfer to J
  2. J back transfer to I (ack not relayed)
  3. I re-transfer to J (2nd ack not relayed

When doing 3. it ended up NFT being escrowed on both sides. Instead of burning during transfer, but escrow it first and then burns in on ACK succuss. User could have gone only through step 2 - without self-relaying. This is the root of cause. In other case when getting stuck doing step 3, the root of cause is not obvious and much harder to find!

@taitruong
Copy link
Contributor Author

There are currently 3 solutions for solving this so far:

(Cool stuff btw, from @gjermundgaraba! Hope I was summing it up correctly :) )

And our proposal for a governed ICS721 environment by whitelisting collections and channels: #714
The 2 contracts we have provided doesn't allow any malicious collection or channel to do transfers for this governed ICS721 environment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ics-721 Possible protocol vulnerability
Projects
None yet
Development

No branches or pull requests

6 participants