20,000 USDC
View results
Submission Details
Severity: high

Arbitrary from in transferFrom

Summary

The transferFrom function in the setPool function uses an arbitrary address p.lender to transfer tokens from. If p.lender has not approved the contract to spend tokens on their behalf, the transferFrom will fail due to insufficient allowance. Ensure p.lender has called the approve function on the p.loanToken contract to set a sufficient allowance for the contract before calling the setPool function.

Alice approves this contract to spend her ERC20 tokens. Bob can call a and specify Alice's address as the from parameter in transferFrom, allowing him to transfer Alice's tokens to himself.

Vulnerability Details

Lender.setPool(Pool) src/Lender.sol Lines #130-176; specifically #152-156 IERC20(p.loanToken).transferFrom

function setPool(Pool calldata p) public returns (bytes32 poolId) {
// validate the pool
if (
p.lender != msg.sender ||
p.minLoanSize == 0 ||
p.maxLoanRatio == 0 ||
p.auctionLength == 0 ||
p.auctionLength > MAX_AUCTION_LENGTH ||
p.interestRate > MAX_INTEREST_RATE
) revert PoolConfig();
// check if they already have a pool balance
poolId = getPoolId(p.lender, p.loanToken, p.collateralToken);
// you can't change the outstanding loans
if (p.outstandingLoans != pools[poolId].outstandingLoans)
revert PoolConfig();
uint256 currentBalance = pools[poolId].poolBalance;
if (p.poolBalance > currentBalance) {
// if new balance > current balance then transfer the difference from the lender
152: IERC20(p.loanToken).transferFrom(
p.lender,
address(this),
p.poolBalance - currentBalance
);
} else if (p.poolBalance < currentBalance) {
// if new balance < current balance then transfer the difference back to the lender
IERC20(p.loanToken).transfer(
p.lender,
currentBalance - p.poolBalance
);
}
emit PoolBalanceUpdated(poolId, p.poolBalance);
if (pools[poolId].lender == address(0)) {
// if the pool doesn't exist then create it
emit PoolCreated(poolId, p);
} else {
// if the pool does exist then update it
emit PoolUpdated(poolId, p);
}
pools[poolId] = p;
}

Impact

Alice approves this contract to spend her ERC20 tokens. Bob can call a and specify Alice's address as the from parameter in transferFrom, allowing him to transfer Alice's tokens to himself.

Tools Used

Manual Review.
Slither reference: https://github.com/crytic/slither/wiki/Detector-Documentation#arbitrary-from-in-transferfrom

Recommendations

Use msg.sender as from in transferFrom

Support

FAQs

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