Platform: Code4rena
Start Date: 03/11/2022
Pot Size: $115,500 USDC
Total HM: 17
Participants: 120
Period: 7 days
Judge: LSDan
Total Solo HM: 1
Id: 174
League: ETH
Rank: 111/120
Findings: 2
Award: $13.42
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: 0xdeadbeef0x
Also found by: 8olidity, Ch_301, HE1M, Koolex, Lambda, Nyx, RedOneN, Ruhum, Tomo, Trust, adriro, aphak5010, ayeslick, berndartmueller, brgltd, carlitox477, cccz, codexploder, d3e4, eierina, eighty, immeas, joestakey, lotux, minhquanym, perseverancesuccess, rbserver, rvierdiiev
8.0811 USDC - $8.08
LineLib.receiveTokenOrETH()
can receive ETH and accounts for an amount
to receive but it is possible to send more ETH than amount
, meaning that any extra ETH sent will be lost to the contract.
LineLib.receiveTokenOrETH()
is as follows.
function receiveTokenOrETH( address token, address sender, uint256 amount ) external returns (bool) { if(token == address(0)) { revert TransferFailed(); } if(token != Denominations.ETH) { // ERC20 IERC20(token).safeTransferFrom(sender, address(this), amount); } else { // ETH if(msg.value < amount) { revert TransferFailed(); } } return true; }
In the case ETH is to be received it checks if(msg.value < amount)
, in which case it reverts. We want msg.value == amount
, but if msg.value > amount
, then amount
will be accounted for and msg.value - amount
will be lost.
Code inspection
Change if(msg.value < amount) { revert TransferFailed(); }
into if(msg.value != amount) { revert TransferFailed(); }
.
#0 - c4-judge
2022-11-17T16:43:46Z
dmvt marked the issue as duplicate of #25
#1 - c4-judge
2022-12-06T15:05:24Z
dmvt marked the issue as satisfactory
#2 - C4-Staff
2022-12-20T06:44:54Z
liveactionllama marked the issue as duplicate of #39
🌟 Selected for report: __141345__
Also found by: 0xdeadbeef0x, 8olidity, Amithuddar, Bnke0x0, Ch_301, Deivitto, IllIllI, KingNFT, Nyx, RaymondFam, RedOneN, Satyam_Sharma, SmartSek, Tomo, adriro, bananasboys, carlitox477, cccz, cloudjunky, codexploder, corerouter, cryptonue, d3e4, datapunk, joestakey, martin, merlin, minhquanym, pashov, peanuts, rvierdiiev
5.3388 USDC - $5.34
LineLib.sendOutTokenOrETH()
may revert when its receiver
is a contract, preventing it from receiving ETH, and thus from much of the functionality of the protocol.
LineLib.sendOutTokenOrETH()
is as follows.
/** * @notice - Send ETH or ERC20 token from this contract to an external contract * @param token - address of token to send out. Denominations.ETH for raw ETH * @param receiver - address to send tokens to * @param amount - amount of tokens to send */ function sendOutTokenOrETH( address token, address receiver, uint256 amount ) external returns (bool) { if(token == address(0)) { revert TransferFailed(); } // both branches revert if call failed if(token!= Denominations.ETH) { // ERC20 IERC20(token).safeTransfer(receiver, amount); } else { // ETH payable(receiver).transfer(amount); } return true; }
In the case ETH is to be sent it calls payable(receiver).transfer(amount)
. transfer()
forwards only 2300 gas which may cause a receiver contract to revert if it consumes more than this upon reception, or if gas costs change in the future. See https://consensys.net/diligence/blog/2019/09/stop-using-soliditys-transfer-now/.
Note the comment Send ETH or ERC20 token from this contract to an external contract
, so it is indeed the intention to send ETH to a contract.
Code inspection
Use call()
instead, checking the return value and protecting against reentrancy.
#0 - c4-judge
2022-11-17T16:44:21Z
dmvt marked the issue as duplicate of #14
#1 - c4-judge
2022-12-06T14:39:13Z
dmvt marked the issue as satisfactory
#2 - C4-Staff
2022-12-20T05:56:43Z
liveactionllama marked the issue as duplicate of #369