code-423n4 / 2022-12-tigris-findings

8 stars 4 forks source link

[NAZ-M6] Usage of `send()` Can Result In Revert #590

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-12-tigris/blob/main/contracts/GovNFT.sol#L157

Vulnerability details

Impact

The function crossChain() is used to transfer ETH/WETH to the layerZero endpoint. send() uses a fixed amount of gas, which was used to prevent Reentrancy. However this limit your protocol to interact with others contracts that need more than that to process the transaction.

Proof of Concept

These will inevitably fail when:

  1. The withdrawer smart contract does not implement a payable fallback function.
  2. The withdrawer smart contract implements a payable fallback function which uses more than 2_300 gas units.
  3. The withdrawer smart contract implements a payable fallback function which needs less than 2_300 gas units but is called through a proxy that raises the call’s gas usage above 2_300.

send() uses a fixed amount of gas, which can result in revert. https://consensys.net/diligence/blog/2019/09/stop-using-soliditys-transfer-now/

Tools Used

Manual Review

Recommended Mitigation Steps

Use call instead of send(). Example: (bool succeeded, ) = _to.call{value: _amount}(""); require(succeeded, "Transfer failed.");

GalloDaSballo commented 1 year ago

It's not the send you think it is

c4-judge commented 1 year ago

GalloDaSballo marked the issue as unsatisfactory: Insufficient quality