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

evmtool Returns Long Null String in JSON error Field When Executing REVERT #7608

Closed
Alleysira opened this issue Sep 12, 2024 · 4 comments · Fixed by #7774
Closed

evmtool Returns Long Null String in JSON error Field When Executing REVERT #7608

Alleysira opened this issue Sep 12, 2024 · 4 comments · Fixed by #7774
Assignees
Labels
bug Something isn't working good first issue Good for newcomers P3 Medium (ex: JSON-RPC request not working with a specific client library due to loose spec assumtion)

Comments

@Alleysira
Copy link

Description

Hello developers!

I'm doing fuzzing on EVM implementations, and I'm using besu/evm-tool for testing. I found a log error that when executing the opcode REVERT(0XFD), the getRevertReason() of besu/evm-tool will return a long null string in the error field of json when no revert reason is available, such as \u0000\u0000.

Steps to Reproduce

  1. I try to reproduce this on the latest develop version of besu/evm-tool(6ed1db3). I build evmtool with:
./gradlew --parallel ethereum:evmTool:installDist 
  1. The bytecode to be executed:
# BYTECODE
60255ffd
# MENMONICS
PUSH1 25 //size
PUSH0 //offset
REVERT
  1. Run the besu/evm-tool with:
./ethereum/evmtool/build/install/evmtool/bin/evmtool --code 60255ffd --json

Expected behavior: If no Revert reason is available, no error field is needed.

Actual behavior: evmtool will output meaningless \u0000s.

{"pc":0,"op":96,"gas":"0x2540be400","gasCost":"0x3","memSize":0,"stack":[],"depth":1,"refund":0,"opName":"PUSH1"}
{"pc":2,"op":95,"gas":"0x2540be3fd","gasCost":"0x2","memSize":0,"stack":["0x25"],"depth":1,"refund":0,"opName":"PUSH0"}
{"pc":3,"op":253,"gas":"0x2540be3fb","gasCost":"0x6","memSize":0,"stack":["0x25","0x0"],"depth":1,"refund":0,"opName":"REVERT","error":"\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000"}

{"stateRoot":"0x24ac5f36f02918efac8340b49f4cb41f45ac0bedb8dcfa511b4bfac6da08299a","output":"0x00000000000000000000000000000000000000000000000000000000000000000000000000","gasUsed":"0xb","pass":true,"fork":"Cancun","timens":4076285,"time":4076}

Frequency: Always.

Logs (if a bug)

This is a reference result from geth/evm:

{"pc":0,"op":96,"gas":"0xffffff","gasCost":"0x3","memSize":0,"stack":[],"depth":1,"refund":0,"opName":"PUSH1"}
{"pc":2,"op":95,"gas":"0xfffffc","gasCost":"0x2","memSize":0,"stack":["0x25"],"depth":1,"refund":0,"opName":"PUSH0"}
{"pc":3,"op":253,"gas":"0xfffffa","gasCost":"0x6","memSize":0,"stack":["0x25","0x0"],"depth":1,"refund":0,"opName":"REVERT"}
{"pc":3,"op":253,"gas":"0xfffffa","gasCost":"0x6","memory":"0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000","memSize":64,"stack":[],"depth":1,"refund":0,"opName":"REVERT","error":"execution reverted"}
{"output":"00000000000000000000000000000000000000000000000000000000000000000000000000","gasUsed":"0xb","error":"execution reverted"}

Versions (Add all that apply)

  • Software version: 24.8.0
  • Java version: 22.0.2
  • OS Name & Version: Ubuntu 20.04
  • Virtual Machine software & version: WSL version:2.2.4.0

Thanks for your time!

@siladu siladu added bug Something isn't working good first issue Good for newcomers P3 Medium (ex: JSON-RPC request not working with a specific client library due to loose spec assumtion) labels Sep 16, 2024
@itfat
Copy link
Contributor

itfat commented Oct 7, 2024

@siladu please assign this issue to me

@shemnon
Copy link
Contributor

shemnon commented Oct 14, 2024

Geth is not conforming to https://eips.ethereum.org/EIPS/eip-3155 - which is the spec the standard trace is following. The optional field "error" should contain the revert reason, not text stating a revert occured. Or the field should not be present in Geth as the field is optional.

@Alleysira
Copy link
Author

Thanks for your reply, @shemnon.
I checked Geth code and there are indeed revert reason related implementation, but I can't trigger this even with non-empty memory.
This is the test case that fills the return data with ff.

$ ./evm --code 7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff60005260205ffd --json --nomemory=false run
{"pc":0,"op":127,"gas":"0x2540be400","gasCost":"0x3","memSize":0,"stack":[],"depth":1,"refund":0,"opName":"PUSH32"}
{"pc":33,"op":96,"gas":"0x2540be3fd","gasCost":"0x3","memSize":0,"stack":["0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"],"depth":1,"refund":0,"opName":"PUSH1"}
{"pc":35,"op":82,"gas":"0x2540be3fa","gasCost":"0x6","memSize":0,"stack":["0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff","0x0"],"depth":1,"refund":0,"opName":"MSTORE"}
{"pc":36,"op":96,"gas":"0x2540be3f4","gasCost":"0x3","memory":"0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff","memSize":32,"stack":[],"depth":1,"refund":0,"opName":"PUSH1"}
{"pc":38,"op":95,"gas":"0x2540be3f1","gasCost":"0x2","memory":"0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff","memSize":32,"stack":["0x20"],"depth":1,"refund":0,"opName":"PUSH0"}
{"pc":39,"op":253,"gas":"0x2540be3ef","gasCost":"0x0","memory":"0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff","memSize":32,"stack":["0x20","0x0"],"depth":1,"refund":0,"opName":"REVERT"}
{"pc":39,"op":253,"gas":"0x2540be3ef","gasCost":"0x0","memory":"0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff","memSize":32,"stack":[],"depth":1,"refund":0,"opName":"REVERT","error":"execution reverted"}
{"output":"ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff","gasUsed":"0x11","error":"execution reverted"}

However, evmtool will still outout non-ASCii characters in the output with the same input, I think this is not the 'revert-reason' we wanted, which is not a readable Description of an error in the spec eip.

./ethereum/evmtool/build/install/evmtool/bin/evmtool --code 3d7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff60005260205ffd --json
{"pc":0,"op":61,"gas":"0x2540be400","gasCost":"0x2","memSize":0,"stack":[],"depth":1,"refund":0,"opName":"RETURNDATASIZE"}
{"pc":1,"op":127,"gas":"0x2540be3fe","gasCost":"0x3","memSize":0,"stack":["0x0"],"depth":1,"refund":0,"opName":"PUSH32"}
{"pc":34,"op":96,"gas":"0x2540be3fb","gasCost":"0x3","memSize":0,"stack":["0x0","0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"],"depth":1,"refund":0,"opName":"PUSH1"}
{"pc":36,"op":82,"gas":"0x2540be3f8","gasCost":"0x6","memSize":0,"stack":["0x0","0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff","0x0"],"depth":1,"refund":0,"opName":"MSTORE"}
{"pc":37,"op":96,"gas":"0x2540be3f2","gasCost":"0x3","memSize":32,"stack":["0x0"],"depth":1,"refund":0,"opName":"PUSH1"}
{"pc":39,"op":95,"gas":"0x2540be3ef","gasCost":"0x2","memSize":32,"stack":["0x0","0x20"],"depth":1,"refund":0,"opName":"PUSH0"}
{"pc":40,"op":253,"gas":"0x2540be3ed","gasCost":"0x0","memSize":32,"stack":["0x0","0x20","0x0"],"depth":1,"refund":0,"opName":"REVERT","error":"��������������������������������"}
��������������������������������
{"stateRoot":"0xd77d59d1544f33b6d97376fcbb5c0f052162dcb00f05d71e383bf68e16f64718","output":"0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff","gasUsed":"0x13","pass":true,"fork":"Cancun","timens":5301517,"time":5301}

I checked with hexdump, the non-ASCii characters are:

00003f0 5245 2254 222c 7265 6f72 2272 223a bfef
0000400 efbf bfbf bfef efbf bfbf bfef efbf bfbf
0000410 bfef efbf bfbf bfef efbf bfbf bfef efbf
0000420 bfbf bfef efbf bfbf bfef efbf bfbf bfef
0000430 efbf bfbf bfef efbf bfbf bfef efbf bfbf
0000440 bfef efbf bfbf bfef efbf bfbf bfef efbf
0000450 bfbf bfef efbf bfbf bfef efbf bfbf 7d22
0000460 ef0a bdbf bfef efbd bdbf bfef efbd bdbf
0000470 bfef efbd bdbf bfef efbd bdbf bfef efbd
0000480 bdbf bfef efbd bdbf bfef efbd bdbf bfef
0000490 efbd bdbf bfef efbd bdbf bfef efbd bdbf
00004a0 bfef efbd bdbf bfef efbd bdbf bfef efbd
00004b0 bdbf bfef efbd bdbf bfef efbd bdbf bfef
00004c0 0abd 227b 7473 7461 5265 6f6f 2274 223a

Thanks for your patience!

@shemnon
Copy link
Contributor

shemnon commented Oct 14, 2024

Yea, besu needs to make it's reverts hex encoded.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers P3 Medium (ex: JSON-RPC request not working with a specific client library due to loose spec assumtion)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants