Submission Details
Severity:
Ignored Return Value in transfer and transferFrom Function
Summary
the smart contract ignores the return value of the transferFrom function call, which can lead to potential vulnerabilities and issues. Ignoring the return value may prevent the contract from detecting transaction failures, leaving it unaware of failed token transfers and other potential problems.
Vulnerability Details
all the transfer and transferFrom functions within the project fails to check for the return value which could lead to issues detecting transaction failures and leaving the project unaware of the token transfers
Impact
function addToPool(bytes32 poolId, uint256 amount) 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 lender to the contract
// @audit-issue execution status check - this can fail
IERC20(pools[poolId].loanToken).transferFrom(
msg.sender,
address(this),
amount
);
}
function removeFromPool(bytes32 poolId, uint256 amount) 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
// @audit-issue execution status check - this can fail
IERC20(pools[poolId].loanToken).transfer(msg.sender, amount);
// @audit-issue emit event not emitted at the right place
}
function borrow(Borrow[] calldata borrows) public {
// @audit-issue no array length check, can lead to DOS attacks
for (uint256 i = 0; i < borrows.length; i++) {
bytes32 poolId = borrows[i].poolId;
uint256 debt = borrows[i].debt;
uint256 collateral = borrows[i].collateral;
// get the pool info
Pool memory pool = pools[poolId];
// make sure the pool exists
if (pool.lender == address(0)) revert PoolConfig();
// validate the loan
if (debt < pool.minLoanSize) revert LoanTooSmall();
if (debt > pool.poolBalance) revert LoanTooLarge();
if (collateral == 0) revert ZeroCollateral();
// make sure the user isn't borrowing too much
uint256 loanRatio = (debt * 10 ** 18) / collateral;
if (loanRatio > pool.maxLoanRatio) revert RatioTooHigh();
// create the loan
Loan memory loan = Loan({
lender: pool.lender,
borrower: msg.sender,
loanToken: pool.loanToken,
collateralToken: pool.collateralToken,
debt: debt,
collateral: collateral,
interestRate: pool.interestRate,
startTimestamp: block.timestamp,
auctionStartTimestamp: type(uint256).max,
auctionLength: pool.auctionLength
});
// update the pool balance
_updatePoolBalance(poolId, pools[poolId].poolBalance - debt);
pools[poolId].outstandingLoans += debt;
// calculate the fees
uint256 fees = (debt * borrowerFee) / 10000; // converting this to percentage
// transfer fees
IERC20(loan.loanToken).transfer(feeReceiver, fees);
// transfer the loan tokens from the pool to the borrower
IERC20(loan.loanToken).transfer(msg.sender, debt - fees);
// transfer the collateral tokens from the borrower to the contract
IERC20(loan.collateralToken).transferFrom(
msg.sender,
address(this),
collateral
);
loans.push(loan);
emit Borrowed(
msg.sender,
pool.lender,
loans.length - 1,
debt,
collateral,
pool.interestRate,
block.timestamp
);
}
}
function repay(uint256[] calldata loanIds) public {
// @audit-issue no array length check, can lead to DOS attacks
for (uint256 i = 0; i < loanIds.length; i++) {
uint256 loanId = loanIds[i];
// get the loan info
Loan memory loan = loans[loanId];
// calculate the interest
(
uint256 lenderInterest,
uint256 protocolInterest
) = _calculateInterest(loan);
bytes32 poolId = getPoolId(
loan.lender,
loan.loanToken,
loan.collateralToken
);
// update the pool balance
_updatePoolBalance(
poolId,
pools[poolId].poolBalance + loan.debt + lenderInterest
);
pools[poolId].outstandingLoans -= loan.debt;
// @audit-issue no txn status checks and multiple transfer functions
// transfer the loan tokens from the borrower to the pool
IERC20(loan.loanToken).transferFrom(
msg.sender,
address(this),
loan.debt + lenderInterest
);
// transfer the protocol fee to the fee receiver
IERC20(loan.loanToken).transferFrom(
msg.sender,
feeReceiver,
protocolInterest
);
// transfer the collateral tokens from the contract to the borrower
IERC20(loan.collateralToken).transfer(
loan.borrower,
loan.collateral
);
emit Repaid(
msg.sender,
loan.lender,
loanId,
loan.debt,
loan.collateral,
loan.interestRate,
loan.startTimestamp
);
// delete the loan
delete loans[loanId];
}
}
and multiple other places
Tools Used
slither and manual review
Recommendations
make sure to correctly check for the statuses of the funtion call since we are inheriting from IERC20 interface which specifies transfer and transferFrom returns bool
Prize pool breakdown
Total prize
20,000 USDC
nSLOC
70628 USDC / LOC
18,000 USDC
2,000 USDC
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.