20,000 USDC
View results
Submission Details
Severity: high

Result of transfer / transferFrom not checked

Summary

Not checking the return value of transfer and transferFrom methods executed on ERC20 tokens can disrupt the entire accounting of the system.

Vulnerability Details

IERC20(tokenAddress).transfer() and IERC20(tokenAddress).transferFrom() are used throughout the contracts to move funds without checking for the return value. One example for each is shown below:

if (p.poolBalance > currentBalance) {
// if new balance > current balance then transfer the difference from the lender
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
);
}

Some tokens can return false instead of reverting in case of a failed transfer.

Impact

The entire accounting will be invalid. Users can create pools, borrow and repay loans without actually holding and transferring the tokens.

Tools Used

Manual review

Recommendations

Checking the return value or use a standard implementation like Openzeppelin's SafeERC20.

if (p.poolBalance > currentBalance) {
// if new balance > current balance then transfer the difference from the lender
bool success = IERC20(p.loanToken).transferFrom(
p.lender,
address(this),
p.poolBalance - currentBalance
);
require(success,"failed transfer");
} else if (p.poolBalance < currentBalance) {
// if new balance < current balance then transfer the difference back to the lender
bool success = IERC20(p.loanToken).transfer(
p.lender,
currentBalance - p.poolBalance
);
require(success,"failed transfer");
}

Support

FAQs

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

Give us feedback!