The Standard

Summary

Return value of "transfer" is not checked

Vulnerability Details

According to Openzeppelin's EIP-20, return value of a transfer should be checked. This would show if a transfer is a success or a failure.

However, in the claimRewards function, it uses the "transfer" keyword without checking the return statement. That is, it doesn't check if a transfer succeeds or fails.

A transfer can fail silently for different reasons. When this happens, the LiquidationPool contract (the reward-sending contract) will not revert. The contract would delete a user's reward (whereas the reward is still in the contract) and deem a user to have claimed the reward. This is how the claimRewards function is coded.

Impact

A user can lose his rewards forever - even though the reward is still in the contract.

Tools Used

Manual review

Recommendations

Use safeTransfer instead of transfer