20,000 USDC
View results
Submission Details
Severity: high

`Lender` does not check if the `token.transfer` and `token.transferFrom` operation have been executed successfully

Summary

The Lender contract does not check if any of the token.transfer or token.transferFrom operation has been correctly executed.

This could lead to accounting errors that would store an incorrect value inside the following state variables:

  • loan.debt

  • loan.collateral

  • pool.poolBalance

  • pool.outstandingLoans

If the transfer fails, it could lead to reverts on the Lender operations or token loss.

Vulnerability Details

The Lender contract does not check if any of the token.transfer or token.transferFrom operation has been correctly executed.

This could lead to accounting errors that would store an incorrect value inside the following state variables:

  • loan.debt

  • loan.collateral

  • pool.poolBalance

  • pool.outstandingLoans

If the transfer fails, it could lead to reverts on the Lender operations or token loss.

Let's make an example:

  1. Alice opens a Pool that would deposit 100 token of lendingToken

  2. The lendingToken.transferFrom operation fails but the Lender contract store pool.poolBalance = 100 anyway. Because the operation has failed, those 100 tokens are still in the Alice's personal balance and have not been transferred to the Lender contract.

  3. Alice can call lender.removeFromPool(poolId, 100) and remove 100 tokens from the Lender contract that could have been deposited by another lender/borrower in the past.

Impact

By not correctly handling the result of the execution of the transfer and transferFrom operation, the Lender contract could wrongly account for some internal state variables of the pool and loan structure.

If this happens, it could lead to reverts on Lender operations or possible fund loss (see example above)

Tools Used

Manual

Recommendations

The Lender contract should use OpenZeppelin SafeERC20 to perform the transfer and transferFrom operation.

SafeERC20 offer Wrappers around ERC20 operations that throw on failure (when the token contract returns false). Tokens that return no value (and instead revert or throw on failure) are also supported, non-reverting calls are assumed to be successful.

After adding the support to SafeERC20 every transfer and transferFrom calls should be replaced by safeTransfer and safeTransferFrom

Support

FAQs

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

Give us feedback!