TokenManagerERC20.sol uses transferFrom() instead of safeTransferFrom() #8
Labels
2 (Med Risk)
Assets not at direct risk, but function/availability of the protocol could be impacted or leak value
bug
Something isn't working
disagree with severity
Sponsor confirms validity, but disagrees with warden’s risk assessment (sponsor explain in comments)
sponsor disputed
Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue
Lines of code
https://github.com/skalenetwork/ima-c4-audit/blob/main/contracts/schain/TokenManagers/TokenManagerERC20.sol#L298
https://github.com/skalenetwork/ima-c4-audit/blob/main/contracts/schain/TokenManagers/TokenManagerERC20.sol#L303
Vulnerability details
Impact
In TokenManagerERC20.sol the _exit() function makes use of transferFrom() instead of using safeTransferFrom(). Tokens that don’t correctly implement the latest EIP20 spec will be unusable in the protocol as they revert the transaction because of the missing return value.
Proof of Concept
https://github.com/skalenetwork/ima-c4-audit/blob/main/contracts/schain/TokenManagers/TokenManagerERC20.sol#L298
https://github.com/skalenetwork/ima-c4-audit/blob/main/contracts/schain/TokenManagers/TokenManagerERC20.sol#L303
Tools Used
Manual code review
Recommended Mitigation Steps
It's recommended to use OpenZeppelin’s SafeERC20 versions with the safeTransfer and safeTransferFrom functions that handle the return value check as well as non-standard-compliant tokens.
The text was updated successfully, but these errors were encountered: