Summary

The contract exhibits unsafe interactions with ERC20 tokens, potentially leading to unexpected behaviors. Specifically, it directly calls methods like transfer, transferFrom, and others without using a safeERC20 wrapper, which can mitigate issues arising from non-standard ERC20 token implementations. Additionally, the contract does not check the return values of these methods, assuming their success. This omission can be problematic, especially with tokens that return false on failure instead of reverting. Such unchecked calls can lead to silent failures, where the contract believes a transfer was successful when it wasn't. Combining these vulnerabilities can expose the contract to risks when interacting with certain ERC20 tokens. It's crucial to ensure that token interactions are both safe from non-standard behaviors and reliably checked for success.

Vulnerability Details

• Unsafe ERC20 Interactions:

The contract directly interacts with ERC20 tokens without using a safeERC20 wrapper. This can be problematic with non-standard ERC20 tokens that don't revert on failure.

Similarly, in multiple functions like repay, giveLoan, buyLoan, and others, direct calls to transfer and transferFrom are made without safety checks.

• Lack of Transfer Success Checks:

The contract assumes that all ERC20 transfer and transferFrom calls will succeed without checking their return values. Some ERC20 tokens return false instead of reverting on failure, which can lead to silent failures.

--

These patterns of direct ERC20 interactions without safety wrappers and the lack of success checks can expose the contract to risks when dealing with certain ERC20 tokens. It's essential to ensure that every token interaction is both safe from non-standard behaviors and reliably checked for success.

Impact

• Loss of Funds: If a non-standard ERC20 token doesn't revert on failure but instead returns false, the contract would continue execution as if the transfer was successful. This could lead to an inconsistent state where the contract believes it has more tokens than it does, potentially leading to loss of funds.
  
  
  

• Inconsistent Contract State: The contract's state could become inconsistent if a token transfer fails but is not detected by the contract. This could lead to incorrect accounting of balances and loans, which could disrupt the normal operation of the contract and potentially lead to loss of funds.
  
  
  

• Unexpected Behavior with Non-Standard Tokens: Non-standard ERC20 tokens that have transfer and transferFrom functions with non-standard behavior could cause the contract to behave unexpectedly. This could lead to a variety of issues, including the ones mentioned above, and could potentially be exploited by malicious actors.

Tools Used

VSCode, Slither

Recommendations

• Use OpenZeppelin's SafeERC20 Library: OpenZeppelin's SafeERC20 library provides functions that wrap the original ERC20 transfer, transferFrom, and approve functions. These functions revert the transaction if the underlying call fails, preventing the contract from continuing execution in an inconsistent state.

• Check Return Values of Token Transfers: Always check the return value of transfer and transferFrom calls. If these functions return false, the contract should revert the transaction to prevent state inconsistencies.