code-423n4 / 2023-09-delegate-findings

2 stars 1 forks source link

Fee on transfer tokens will cause users to lose funds #296

Closed c4-submissions closed 1 year ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-09-delegate/blob/main/src/DelegateToken.sol#L297 https://github.com/code-423n4/2023-09-delegate/blob/main/src/libraries/DelegateTokenRegistryHelpers.sol#L253 https://github.com/code-423n4/2023-09-delegate/blob/main/src/DelegateToken.sol#L399

Vulnerability details

Impact

Some ERC20 tokens allow for charging a fee any time transfer() or transferFrom() is called. If a contract does not allow for amounts to change after transfers, subsequent transfer operations based on the original amount will revert() due to the contract having an insufficient balance.

Proof of Concept

Tools Used

None

Recommended Mitigation Steps

    function pullERC20AfterCheck(address underlyingContract, uint256 pullAmount) internal returns(uint256) {
        uint256 balanceBefore = IERC20(underlyingContract).balanceOf(address(this));
        SafeERC20.safeTransferFrom(IERC20(underlyingContract), msg.sender, address(this), pullAmount);
        uint256 balanceAfter = IERC20(underlyingContract).balanceOf(address(this));
        return balanceBefore - balanceAfter;
    }

Assessed type

Token-Transfer

GalloDaSballo commented 1 year ago

OSS

c4-judge commented 1 year ago

GalloDaSballo marked the issue as unsatisfactory: Out of scope