EOSIO / eos

An open source smart contract platform
https://developers.eos.io/manuals/eos
MIT License
11.27k stars 3.6k forks source link

push transaction json bad chars in memo #5740

Closed Vasiliy-Bondarenko closed 5 years ago

Vasiliy-Bondarenko commented 6 years ago
{"expiration":"2018-09-19T07:36:10","ref_block_num":35503,"ref_block_prefix":927242102,"max_net_usage_words":0,"max_cpu_usage_ms":0,"delay_sec":0,"context_free_actions":[],"actions":[{"account":"eosio.token","name":"transfer","authorization":[{"actor":"bxdeposit555","permission":"active"}],"data":{"from":"bxdeposit555","to":"withdrawals1","quantity":"0.0001 EOS","memo":"Hello\u0000world"}}],"transaction_extensions":[],"signatures":["SIG_K1_KmPtdBHVJHWPeuK9JX51xxCNVyUPjw27d41MHPRa5zKoCUvofgyodsPQvVJHRFsGPkGkbwiCBTD13VAtjvz9VHvyeAGRR2"]}

this is a valid json-encoded transaction with a null character in memo. and it fails when trying to broadcast it...

cleos push transaction --skip-sign '{"expiration":"2018-09-19T07:36:10","ref_block_num":35503,"ref_block_prefix":927242102,"max_net_usage_words":0,"max_cpu_usage_ms":0,"delay_sec":0,"context_free_actions":[],"actions":[{"account":"eosio.token","name":"transfer","authorization":[{"actor":"bxdeposit555","permission":"active"}],"data":{"from":"bxdeposit555","to":"withdrawals1","quantity":"0.0001 EOS","memo":"Hello\u0000world"}}],"transaction_extensions":[],"signatures":["SIG_K1_KmPtdBHVJHWPeuK9JX51xxCNVyUPjw27d41MHPRa5zKoCUvofgyodsPQvVJHRFsGPkGkbwiCBTD13VAtjvz9VHvyeAGRR2"]}'
Error 3090003: Provided keys, permissions, and delays do not satisfy declared authorizations
Ensure that you have the related private keys inside your wallet and your wallet is unlocked.

so there is some bug while decoding json or reading data afterwards which prevents from reaching signatures.

taokayan commented 6 years ago

The error message indicates the signature is incorrect. What's your version of nodeos/cleos? Did you try to sign using invalid private key?

Vasiliy-Bondarenko commented 6 years ago

@taokayan signature is fine. when i create the same transaction with "usual" memo - i am able to broadcast it easily.

$ cleos version client
Build version: 08819aae
$ cleos get info
{
  "server_version": "08819aae",
  "chain_id": "cf057bbfb72640471fd910bcb67639c22df9f92470936cddc1ade0e2f2e7dc4f",
  "head_block_num": 180112,
  "last_irreversible_block_num": 180111,
  "last_irreversible_block_id": "0002bf8f8daa371cbd115d3fd309c506867c349fd24fde56257969c371b247d2",
  "head_block_id": "0002bf904a1620459d48314c414ddf64ce510987877f2a4e4d06ebac6beada7d",
  "head_block_time": "2018-09-19T08:28:58.000",
  "head_block_producer": "eosio",
  "virtual_block_cpu_limit": 200000000,
  "virtual_block_net_limit": 1048576000,
  "block_cpu_limit": 199900,
  "block_net_limit": 1048576,
  "server_version_string": "v1.2.5"
}
tbfleming commented 6 years ago

Not a bug. Strings are for text, not binary data.

wanderingbort commented 6 years ago

It does seem like this should be a valid json transaction. The null character is properly escaped the same way any escaped Unicode would be. I'm not sure I expect the contracts to behave properly but our binary serialization should

On Wed, Sep 19, 2018, 7:54 AM Todd Fleming notifications@github.com wrote:

Not a bug. Strings are for text, not binary data.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/EOSIO/eos/issues/5740#issuecomment-422773256, or mute the thread https://github.com/notifications/unsubscribe-auth/ACYR4kVq1JiVJHRBuECftb15uMLqYKfyks5ucjCLgaJpZM4Wvip- .

Vasiliy-Bondarenko commented 6 years ago

@tbfleming it's escaped. so it's a text data. @wanderingbort absolutely agree. if software considers it a bad memo - error should indicate that, not other non-related error.

taokayan commented 6 years ago

I think the limitation is that we don't treat "\u0000" as binary 0. Instead "\u0000" is actually treaded as 'u' + '0' + '0' + '0' + '0', which results in different signature. So the root cause is that the format of "\u" + 4 digit hex number is not parsed properly according to JSON standard(http://www.json.org/).

./cleos push action -j -d eosio.token transfer '["a123","eosio","10.0000 SYS","h\u0000abc"]'
{
  "expiration": "2018-09-19T07:18:14",
  "ref_block_num": 589,
  "ref_block_prefix": 1638585869,
  "max_net_usage_words": 0,
  "max_cpu_usage_ms": 0,
  "delay_sec": 0,
  "context_free_actions": [],
  "actions": [{
      "account": "eosio.token",
      "name": "transfer",
      "authorization": [],
      "data": "00000000003044300000000000ea3055a086010000000000045359530000000009687530303030616263"
    }
  ],
  "transaction_extensions": [],
  "signatures": [],
  "context_free_data": []
}
Vasiliy-Bondarenko commented 6 years ago

I dont think so. Eosio uses null terminated string here. So it thinks that memo string has ended before it's actual end. I'm not profficient in cpp, this is just my observation :) Solution: use buffer or some other string object with explicit length set.

tbfleming commented 5 years ago

JSON supports \u0000 in strings, but many things which which support JSON don't. e.g. PostgreSQL bans \u0000 in JSON strings, conditionally supports other escapes, and fully supports Unicode characters which aren't escaped.

taokayan commented 5 years ago

looks like converting \u0000 to binary 0 might make thing more tricky to handle or results in security loopholes. I guess we'd better keep the current behaviour (treat \u0000 as '\u' + '0' + '0' + '0').

taokayan commented 5 years ago

Please ask in eosio.stackexchange.com if you have further questions.

Shrey33 commented 5 years ago

@Vasiliy-Bondarenko can you tell me how to reproduce it ?

Vasiliy-Bondarenko commented 5 years ago

@Shrey33 i think i was just constructing TX with eosjs library. and broadcasting with cleos after that. just set memo to "Hello\u0000world"