Non-safe version of transfer function used #383
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
duplicate
This issue or pull request already exists
Lines of code
https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/contracts/enforcer/PA1D.sol#L426
Vulnerability details
Impact
Detailed description of the impact of this finding.
The function uses a for loop to send token to each address. However, the transfer function is not safe for many ERC-20 tokens as as it might or might not return a value depending on the implementation. A safe version is recommended.
Proof of Concept
Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.
https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/contracts/enforcer/PA1D.sol#L426
Tools Used
Remix
Recommended Mitigation Steps
Use a safe version of Transfer function.
https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC20/utils/SafeERC20.sol
Also recommend to use a pull-over-push pattern, let the user withdraw the token rather than sending the token to them.
Otherwise, if one transfer fail and the whole transaction has to revert, then all other people will be affected.
The text was updated successfully, but these errors were encountered: