JS tracer log.memory object does not handle uninitialized memory correctly

[danielmh0]

System information

Geth version: 1.10.19-stable
OS & Version: Windows/Linux/OSX

Expected behaviour

In debug_traceTransaction, with a custom JavaScript tracer, accessing log.memory.slice() on uninitialized memory should return zero'd bytes.

Actual behaviour

There is an error Tracer access out of bound memory from https://sourcegraph.com/github.com/ethereum/go-ethereum/-/blob/eth/tracers/js/goja.go?L453.

Some smart contracts make CALLs pointing to uninitialized (0) memory, but geth will not expand the memory buffer until after the CALL is executed. But the tracer needs to access the memory before the CALL, in order to extract input data (particularly in case the CALL eventually fails).

Steps to reproduce the behaviour

Here is a mainnet transaction that refers to unininitalized memory for a CALL's input data: https://etherscan.io/tx/0x65546ff3a5ff18b06226d1f1af4f9386d9dfd8a0e37afd1871ffa73a863bbd58

To reproduce, use the following simple tracer:

$ curl -H 'Content-Type: application/json' -X POST --data '{"jsonrpc":"2.0","method":"debug_traceTransaction","params":["0x65546ff3a5ff18b06226d1f1af4f9386d9dfd8a0e37afd1871ffa73a863bbd58", {"tracer": "{ step(log,db){ if(log.op.toString() === '"'"'CALL'"'"') { const inputOffset = log.stack.peek(3).valueOf(); const inputLength = log.stack.peek(4).valueOf();  const inputEnd = inputOffset + inputLength; log.memory.slice(inputOffset, inputLength); } }, fault(log,db){}, result(){ return '"'"'success'"'"'; } }"}] ,"id":"test"}' GETH_ARCHIVE_HOST:8544

Result on erigon:

{"jsonrpc":"2.0","id":"test","result":"success"}

Result on geth:

{"jsonrpc":"2.0","id":"test","error":{"code":-32000,"message":"Tracer accessed out of bound memory: available 288, offset 0, size 304 at step (\u003ceval\u003e:1:212(41))    in server-side tracer function 'step'"}}

[s1na]

Yes there was a change of behaviour as of this PR: #24867 where the tracer now reports memory pre-expansion. This was done for consistency reasons. Everything else the tracer emitted was for pre-opcode-execution and memory expansion.

I also noticed recently this causes the callTracerLegacy to fail on various mainnet txes with the same error. But callTracer works which is the recommended tracer.

Have you considered using the enter hook to get the calldata? This is a recent feature. You can see the docs here: https://geth.ethereum.org/docs/rpc/ns-debug#enter--exit. For example:

https://github.com/s1na/go-ethereum/blob/a1e77af4182054684dae9cbe148df973f07184c4/eth/tracers/internal/tracers/call_tracer_js.js#L54

[danielmh0]

enter would be good for this, yes, but I'd like to keep a single tracer compatible with old+new versions of geth, as well as erigon.

This is still technically a bug, because the bytes are defined to be 0 even before they are read/written by the EVM, right? If we don't want to modify the actual buffer, the tracer hook can just pad 0s?

[s1na]

@holiman What do you say? on the one hand we have memory pre-allocation, and on the other getting a out-of-bounds slice of memory panics.

My gut says padding memory in the tracer with zeros might hide user mistakes?

[karalabe]

I think the consensus was to allow OOB accesses (within some limits, e.g. 1MB) and pad it with zeroes. It's the simplest and is consistent with how EVM initializes memory.