20,000 USDC
View results
Submission Details
Severity: medium
Valid

If a borrower or lender got blacklisted by asset contract, their collateral or loan funds can be permanently frozen with the pool

Summary

It's impossible for borrower or lender to transfer their otherwise withdraw-able funds to another address. If for some reason borrower or kicker got blacklisted by collateral or loan token contract (correspondingly), these funds will be permanently frozen as now there is no mechanics to move them to another address or specify the recipient for the transfer.

Vulnerability Details

If during the duration of a loan the borrower or lender got blacklisted by collateral asset contract, let's say it is USDC, there is no way to retrieve the collateral. These collateral funds will be permanently locked at the Pool contract balance.

This happens in repay function (which affects the borrower) and seizeLoan function (which affects the lender). There is no option to specify the address receiving the collateral asset.

For loan asset's case (which affects the lender) there is a withdrawal possibility of loan funds via Lender.sol's removeFromPool(), but for the lender there is no way to transfer funds due to ownership or even specify transfer recipient, so the corresponding loan funds will be frozen with the pool if current lender is blacklisted.

Impact

Principal funds of borrower or lender can be permanently frozen in full, but blacklisting is a low probability event, so setting the severity to be medium.

Tools Used

Manual

Recommendations

Consider adding the recipient argument to the repay(), seizeLoan() and removeFromPool() functions, so the balance beneficiary msg.sender can specify what address should receive the funds, for example:

https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Lender.sol#L198

- function removeFromPool(bytes32 poolId, uint256 amount) external {
+ function removeFromPool(bytes32 poolId, uint256 amount, address recipient) external {
if (pools[poolId].lender != msg.sender) revert Unauthorized();
if (amount == 0) revert PoolConfig();
_updatePoolBalance(poolId, pools[poolId].poolBalance - amount);
// transfer the loan tokens from the contract to the lender
- IERC20(pools[poolId].loanToken).transfer(msg.sender, amount);
+ IERC20(pools[poolId].loanToken).transfer(recipient, amount);
}

Similar modifications for other functions. In the case of the repay() function, which can be call by anyone, allowing setting a recipient would make the collateral asset going straight to that address even if it's not the borrower's address. So, we can make a check here:

https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Lender.sol#L292

- function repay(uint256[] calldata loanIds) public {
+ function repay(uint256[] calldata loanIds, address recipient) public {
...
- IERC20(loan.collateralToken).transfer(
loan.borrower,
loan.collateral
);
+ if (loan.borrower == msg.sender) {
IERC20(loan.collateralToken).transfer(
recipient,
loan.collateral
);
} else {
IERC20(loan.collateralToken).transfer(
loan.borrower,
loan.collateral
);
}
...
}

Support

FAQs

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