`hackenproof` bug in `DecodeResultData`

0x34d commented 1 year ago

Summary of Bug

Backtrace :

--- FAIL: TestDecodeResultData (0.00s)
panic: runtime error: index out of range [0] with length 0 [recovered]
    panic: runtime error: index out of range [0] with length 0

goroutine 90 [running]:
testing.tRunner.func1.2({0x2153bc0, 0xc000f9a048})
    /usr/lib/golang/src/testing/testing.go:1526 +0x24e
testing.tRunner.func1()
    /usr/lib/golang/src/testing/testing.go:1529 +0x39f
panic({0x2153bc0, 0xc000f9a048})
    /usr/lib/golang/src/runtime/panic.go:884 +0x213
github.com/okex/exchain/x/evm/types.UnmarshalEthLogFromAmino({0xc000f821a3, 0x1, 0x1})
    /home/0x34d/project/exchain/x/evm/types/utils.go:248 +0x985
github.com/okex/exchain/x/evm/types.(*ResultData).UnmarshalFromAmino(0xc00069fc68, 0x745?, {0xc000f821a1?, 0x466bb9?, 0x0?})
    /home/0x34d/project/exchain/x/evm/types/utils.go:446 +0x2a5
github.com/okex/exchain/x/evm/types.DecodeResultData({_, _, _})
    /home/0x34d/project/exchain/x/evm/types/utils.go:635 +0xd0
github.com/okex/exchain/x/evm/types.TestDecodeResultData(0x414619?)
    /home/0x34d/project/exchain/x/evm/types/utils_test.go:478 +0x45
testing.tRunner(0xc000fae340, 0x2540f98)
    /usr/lib/golang/src/testing/testing.go:1576 +0x10b
created by testing.(*T).Run
    /usr/lib/golang/src/testing/testing.go:1629 +0x3ea
FAIL    github.com/okex/exchain/x/evm/types 0.049s
FAIL

Steps to Reproduce

file : exchain/x/evm/types/utils_test.go

func TestDecodeResultData(t *testing.T) {

    DecodeResultData([]byte("\x03\x1a\x01O"))
}

run: go test -run TestDecodeResultData $(pwd)/x/evm/types

Impact

Crash over aka DOS on the network.

Note:

Why a crash of a node is not a bug?
This is the same bug i reported on hackenproof.
Usage of DecodeResultData.
For me, this is exactly like: CVE-2018-17145

0x34d commented 1 year ago

And also why crash in an EVM is not a problem?

You Don't read

giskook commented 1 year ago

@0x34d Thanks for your report.

I prodeuce this crash with UT as you point out.
I search the usage of DecodeResultData, it seems RPC will use it.
I deploy a local net which will always panic when the DecodeResultData be called. I found when the panic occurs the response will be call to MyToken.decimals errored: Returned error: {"jsonrpc":"2.0","error":"method handler crashed","id":2914840410484372} but the node runs allright. so I think It will NOT cause a DOS?
as you have reported to hackenproof. How it will be fixed? Is there any sugguestions? Many thanks!

0x34d commented 1 year ago

I deploy a local net which will always panic when the DecodeResultData be called. I found when the panic occurs the response will be call to MyToken.decimals errored: Returned error: {"jsonrpc":"2.0","error":"method handler crashed","id":2914840410484372} but the node runs allright. so I think It will NOT cause a DOS?

It won't; they are both technically different. When DecodeResultData is called by a network, there will be some prefix data, Fuzzing like this is a headache. probably DecodeResultData is not even called with the same data in node.

as you have reported to hackenproof. How it will be fixed? Is there any sugguestions? Many thanks!

something like this. x/evm/types/utils.go:248

        case 9:
            //check if data is not nill
            if dataLen == 0 {
                return nil, fmt.Errorf("invalid removed flag length: %d", dataLen)
            }

0x34d commented 1 year ago

Or maybe you have a crash handler deployed to recover after a crash.

That's how it should be; every major network application recovers after a crash, even those that are written in C.

while true; do
  echo "This will run forever until you manually stop it."
  ./RunMyserver 8080
done

Now you can recover after a crash. There is no such thing as a crash.

okx / exchain