Compound redeems does not check the return value

Summary

Compound does not revert but returns an error code instead.

Vulnerability Detail

Compound’s  functions  return an error code instead of reverting in case of failure: https://docs.compound.finance/v2/ctokens/#error-codes

Converter smart contract never checks for the error codes returned from Compound smart contracts and wraps it in a try/catch. This is not an effective solution and might result in inaccessible funds.

  Safe.transferFrom(IERC20(c), msg.sender, address(this), a);
  ...
  try ICompoundToken(c).redeem(a) {
      // get the balance of underlying assets redeemed
      uint256 balance = IERC20(u).balanceOf(address(this));
      // transfer the underlying back to the user
      Safe.transfer(IERC20(u), msg.sender, balance);
  } catch {
      ...
  }

Example: 1) User tries to redeem tokens where conversion is needed (Pendle, Sense, Apwine). 2) Converter tries to redeem underlying tokens from the Compound but an error code is returned. 3) The execution does not revert but continues. The underlying balance is 0 (because redemption failed) bu the ERC20 transfer of 0 will succeed. 4) Compounding tokens will remain stuck in the Converter.

Impact

If an error occurs, it will continue the execution leaving the user funds in the contract. An admin can release a new converter and set it in the Redeemer contract but this will only affect new conversions, tokens from previous conversions will remain in an old Converter contract.

Code Snippet

https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/Converter.sol#L37

Tool used

Manual Review

Recommendation

Check the return values of Compound tokens. Also, consider introducing a general slippage parameter (min received) in Converter.

Duplicate of #133

sherlock-audit / 2022-10-illuminate-judging

HonorLt - Compound redeems does not check the return value #193