search

code-423n4 / 2022-02-skale-findings

0 stars 0 forks source link

Division by zero when transmitting message array with zero length #41

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/MessageProxyForMainnet.sol#L232-L233

Vulnerability details

Impact

Division by zero will cause EVM to revert in calculating additionalGasPerMessage in MessageProxyForMainnet.postIncomingMessages.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

See https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/MessageProxyForMainnet.sol#L232-L233

Unit test shows 

  Error: VM Exception while processing transaction: reverted with panic code 0x12 (Division or modulo division by zero)

Tools Used

Recommended Mitigation Steps

validate above https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/MessageProxyForMainnet.sol#L232-L233 by 

require(messages.length > 0,  "MsgProxyForMainnet:zero message length");
cstrangedk commented 2 years ago

IMA Agent would never send a transaction with 0 messages, as this is defined in the agent protocol. Further, theoretically if an agent could send a transaction with 0 message, transaction would be simply be reverted and agent would incur gas spend in the attempt.

GalloDaSballo commented 2 years ago

I have to agree with the sponsor that the finding is gas level in nature, the worst case scenario is that the tx will revert after some computation has happened, which doesn't warrant the Med Severity

GalloDaSballo commented 2 years ago

For that reason am downgrading to gas

GalloDaSballo commented 2 years ago

Have given a 0 as this doesn't save gas