Summary

Calls to ERC20::transfer method should always be checked.

Vulnerability Details

Some ERC20 tokens do not revert on failure in transfer but instead return false as a return value (for example ZRX). Because of this it has become a common practice to use OpenZeppelin's SafeERC20 to handle such weird tokens. If transfer fails, but does not revert it can leave tokens stuck in the contract. For example, function LiduidationPool#claimRewards() using non-checked transfer, but if it failed the tokens would get stuck, before its mapping is already deleted:

        if (_rewardAmount > 0) {
            delete rewards[abi.encodePacked(msg.sender, _token.symbol)];    // <----

Impact

Potentially permanently lost (stuck) value for users of the protocol.

Tools Used

Manual review

Recommendations

Using SafeTransfer from OpenZeppelin's SafeERC20 library to handle such tokens