code-423n4 / 2022-03-paladin-findings

0 stars 0 forks source link

Risk of locking funds if the user is a smart contract #67

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-03-paladin/blob/9c26ec8556298fb1dc3cf71f471aadad3a5c74a0/contracts/open-zeppelin/libraries/SafeERC20.sol#L20-L26

Vulnerability details

Impact

Since function overloading is used for stake and unstake, a smart contract which succesfully staked PAL tokens may fail to withdraw, due to safeTransfer method uses transfer with 2300 gas stipend which is not adjustable. In short, unstake function may fail due to shortage of gas

Proof of Concept

     function safeTransfer(
        IERC20 token,
        address to,
        uint256 value
    ) internal {
        _callOptionalReturn(token, abi.encodeWithSelector(token.transfer.selector, to, value));
    }

https://github.com/code-423n4/2022-03-paladin/blob/9c26ec8556298fb1dc3cf71f471aadad3a5c74a0/contracts/open-zeppelin/libraries/SafeERC20.sol#L20-L26

Tools Used

Manual Review

Recommended Mitigation Steps

Team can consider using call.value(amount) after determining with isContract in OpenZeppelin's address.sol

Kogaroshi commented 2 years ago

Cannot reproduce the issue PoC is simply a copy/paste of the SafeERC20 safeTransfer method

Recommended mitigation looks totally irrelevant with the issue raised.

0xean commented 2 years ago

Closing as invalid with not enough information provided by warden.