First Flight #16: Mafia Takedown
First Flight #16
Beginner FriendlyDeFiFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: medium
Invalid

Unsafe ERC20 Operations should not be used

Mill

Summary

ERC20 functions may not behave as expected. For example: return values are not always meaningful. It is recommended to use OpenZeppelin's SafeERC20 library. While this is no problem right now, remember that USDC is a proxy contract and can be upgraded at any time, so it's best practice to be prepared for this case.

Vulnerability Details

Several tokens do not revert in case of failure and return false,transfer/transferFrom will not revert if the transfer fails, and an attacker can call these for free.

3 Found Instances

  • Found in MoneyShelf.sol

    usdc.transferFrom(account, address(this), amount);

  • Found in MoneyShelf.sol

    usdc.transfer(to, amount);

  • Found in src/modules/MoneyVault.sol

    usdc.transfer(to, amount);

Impact

An attacker can call these for free, resulting in free tokens for the attacker and a loss for the protocol.

Tools Used

  • aderyn

  • slither

  • manual review

Recommendations

Use Openzeppelins SafeERC20, or ensure that the transfer/transferFrom return value is checked.

MoneyShelf
+ import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
​
contract MoneyShelf is Shelf {
+ using SafeERC20 for IERC20;
​
​
function depositUSDC(address account, address to, uint256 amount) external {
deposit(to, amount);
- usdc.transferFrom(account, address(this), amount);
+ safeTransferFrom(usdc, account, address(this), amount);
crimeMoney.mint(to, amount);
}
​
​
function withdrawUSDC(address account, address to, uint256 amount) external {
withdraw(account, amount);
crimeMoney.burn(account, amount);
- usdc.transfer(to, amount);
+ safeTransfer(usdc, to, amount);
}
}
MoneyVault
+ import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
​
contract MoneyShelf is Shelf {
+ using SafeERC20 for IERC20;
​
function withdrawUSDC(address account, address to, uint256 amount) external {
require(to == kernel.executor(), "MoneyVault: only GodFather can receive USDC");
withdraw(account, amount);
crimeMoney.burn(account, amount);
- usdc.transfer(to, amount);
+ safeTransfer(usdc, to, amount);
}
}
Updates

Lead Judging Commences

n0kto Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.