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

Incorrect Method of Deleting Array Could Lead to DoS Vulnerability in "Repay" Function

b353n

Summary

This audit report provides an assessment of the code for the "repay" function in the Lender smart contract. The function is intended to delete a loan from the "loans" array once it has been repaid. However, it has been observed that the current implementation does not correctly remove the loan from the array.

Vulnerability Details

The "repay" function in the Lender smart contract attempts to delete the loan from the "loans" array when it is repaid. However, the current implementation is not the correct approach to achieve this. The code snippet below shows the existing deletion method:

// delete the loan
delete loans[loanId];

The above code will not effectively delete the loan from the array.

#Proof of Concept (PoC)
In order to verify the vulnerability, a test was conducted using the "LenderTest" contract. The PoC code attempted to repay a loan and then checked the length of the "loans" array to see if the deletion was successful. The test concluded that the "loans" array still had a length of 1 after the attempted repayment, indicating that the deletion was not effective.

Add function for test propusal in Lender.sol

function getLength() public view returns (uint) {
return loans.length;
}

Code for testing the conception

// 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 LenderTest is Test {
Lender 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 {
lender = new Lender();
loanToken = new TERC20();
collateralToken = new TERC20();
loanToken.mint(address(lender1), 100000 * 10 ** 18);
loanToken.mint(address(lender2), 100000 * 10 ** 18);
collateralToken.mint(address(borrower), 100000 * 10 ** 18);
vm.startPrank(lender1);
loanToken.approve(address(lender), 1000000 * 10 ** 18);
collateralToken.approve(address(lender), 1000000 * 10 ** 18);
vm.startPrank(lender2);
loanToken.approve(address(lender), 1000000 * 10 ** 18);
collateralToken.approve(address(lender), 1000000 * 10 ** 18);
vm.startPrank(borrower);
loanToken.approve(address(lender), 1000000 * 10 ** 18);
collateralToken.approve(address(lender), 1000000 * 10 ** 18);
}
​
function test_borrow() public {
vm.startPrank(lender1);
Pool memory p = Pool({
lender: lender1,
loanToken: address(loanToken),
collateralToken: address(collateralToken),
minLoanSize: 100 * 10 ** 18,
poolBalance: 1000 * 10 ** 18,
maxLoanRatio: 2 * 10 ** 18,
auctionLength: 1 days,
interestRate: 1000,
outstandingLoans: 0
});
bytes32 poolId = lender.setPool(p);
​
(, , , , uint256 poolBalance, , , , ) = lender.pools(poolId);
assertEq(poolBalance, 1000 * 10 ** 18);
​
vm.startPrank(borrower);
Borrow memory b = Borrow({
poolId: poolId,
debt: 100 * 10 ** 18,
collateral: 100 * 10 ** 18
});
Borrow[] memory borrows = new Borrow[](1);
borrows[0] = b;
lender.borrow(borrows);
​
assertEq(loanToken.balanceOf(address(borrower)), 995 * 10 ** 17);
assertEq(collateralToken.balanceOf(address(lender)), 100 * 10 ** 18);
(, , , , poolBalance, , , , ) = lender.pools(poolId);
assertEq(poolBalance, 900 * 10 ** 18);
}
​
function test_repay() public {
test_borrow();
​
bytes32 poolId = keccak256(
abi.encode(
address(lender1),
address(loanToken),
address(collateralToken)
)
);
​
vm.startPrank(borrower);
​
uint256[] memory loanIds = new uint256[](1);
loanIds[0] = 0;
​
loanToken.mint(address(borrower), 5 * 10 ** 17);
​
lender.repay(loanIds);
​
//assertEq(loanToken.balanceOf(address(borrower)), 0);
//assertEq(collateralToken.balanceOf(address(lender)), 0);
//(, , , , uint256 poolBalance, , , , ) = lender.pools(poolId);
//assertEq(poolBalance, 1000 * 10 ** 18);
assertEq(lender.getLength(), 1);
}
}
​

As we can see the loans.lenght is still 1 after we repay the loan.

Impact

The vulnerability identified in the "repay" function could lead to incorrect behavior in the application. Loans that have been repaid will remain in the "loans" array, potentially causing issues with data integrity and affecting subsequent loan operations.

Tools Used

Vscode, foundry

Recommendations

To properly delete the loan from the "loans" array after repayment, the following code can be used:

// Swap the element to be deleted with the last element
loans[loanIds] = myArray[myArray.length - 1];
// Delete the last element
loans.pop();

The recommended code above will effectively remove the loan from the array and maintain its integrity.

Support

FAQs

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