search

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

8 stars 4 forks source link

tradin.sol _handleWithdraw does not verify that tokens were transferred successfully #609

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-12-tigris/blob/588c84b7bb354d20cbca6034544c4faa46e6a80e/contracts/Trading.sol#L676

Vulnerability details

Impact

If for some reason the ERC20 transfer is temporarily failing, the user would totally lose his allocation and funds. All the state variables would already have been updated at this stage, so he can't call withdraw again. There is no way to withdraw these locked tokens.

Proof of Concept

At the last point of withdraw, the function is sending the funds to the user, and does not check the return value - whether it has succeeded:

IERC20(_outputToken).transfer(_trade.trader, IERC20(_outputToken).balanceOf(address(this)) - _balBefore);

Tools Used

mnual review

Recommended Mitigation Steps

add a check that verifies that the transfer has succeede

c4-judge commented 1 year ago

GalloDaSballo marked the issue as unsatisfactory: Out of scope

GalloDaSballo commented 1 year ago

OOS per c4udit report that tells Sponsor to use safeTransfer