code-423n4 / 2024-05-canto-findings

0 stars 0 forks source link

Panic in MustUnmarshal #16

Closed howlbot-integration[bot] closed 5 months ago

howlbot-integration[bot] commented 5 months ago

Lines of code

https://github.com/b-harvest/ethermint/blob/dudong2/feat/cosmos-sdk%40v0.50.x-cometbft%40v0.38.0-2/x/evm/types/msg.go#L464 https://github.com/b-harvest/ethermint/blob/dudong2/feat/cosmos-sdk%40v0.50.x-cometbft%40v0.38.0-2/x/evm/types/msg.go#L471 https://github.com/b-harvest/ethermint/blob/dudong2/feat/cosmos-sdk%40v0.50.x-cometbft%40v0.38.0-2/x/evm/types/msg.go#L478

Vulnerability details

Impact

The ModuleCdc is only strictly used for testing, as stated in https://github.com/b-harvest/ethermint/blob/dudong2/feat/cosmos-sdk%40v0.50.x-cometbft%40v0.38.0-2/x/evm/types/codec.go#L30-L32.

eventhough the usage of ModuleCdc itself is harmless, this can be used to DOS the node by leveraging the fact that MustUnmarshal is being used by ModuleCdc.

MustUnmarshal is very strict and it's going to panic when it fails to unmarshal the data, which we can see in this code https://github.com/cosmos/cosmos-sdk/blob/main/codec/proto_codec.go#L108-L115.

Proof of Concept

example snippet of the POC

"@type": "/ethermint.evm.v1.MsgEthereumTx", "data": { "@type": "/ethermint.evm.v1.DynamicFeeTx", //the data below is the legacyTX fields, However, we defined the @type as a DynamicFeeTx "nonce": "8", "gas_price": "7", "gas": "999999999999", "to": "0x60DD27A3FbB76e158F6e7EE4F1FB926a052CF2ab", "value": "0", "data": "qSEAyw==", "v": "BjY=", "r": "qaJ6tUAIqPQ/neu+fYXEeWoCJtwnssAd9jLbQ2ee/u4=", "s": "JFSlZnGfL/jqBnDjVvmqbGZ+Qt4Mnr03wqRIPKNALtM=" },

Tools Used

manual

Recommended Mitigation Steps

as stated in the https://github.com/cosmos/cosmos-sdk/blob/main/codec/proto_codec.go#L108-L115. to dynamicly handle errors during data unmarshaling, please use UnmarshalInterface function, and handle the error properly, instead of panicking.

Assessed type

DoS

poorphd commented 5 months ago
3docSec commented 5 months ago

Marking as not satisfactory:

c4-judge commented 5 months ago

3docSec marked the issue as unsatisfactory: Insufficient proof