Description

Safety Module and Rewards Manager provide deposit/stake functions. These functions expect that transfer or approval has already been executed prior to a call.

In Depositor(SafetyModule), for example, the function depositReserveAssets is used to deposit with transfer. But, in this function, safeTransferFrom uses the arbitrary parameter from_, not msg.sender. In this case, if there is a user who has approved this contract, the other user could call this function by using from_ as the approved address. If this function is not called with approval in a single transaction, it could be vulnerable to a front-run attack.

  function depositReserveAssets(uint8 reservePoolId_, uint256 reserveAssetAmount_, address receiver_, address from_)
    external
    returns (uint256 depositReceiptTokenAmount_)
  {
    // ...
!   underlyingToken_.safeTransferFrom(from_, address(this), reserveAssetAmount_);

    depositReceiptTokenAmount_ =
      _executeReserveDeposit(reservePoolId_, underlyingToken_, reserveAssetAmount_, receiver_, assetPool_, reservePool_);
  }

Impact

An attacker could execute a front-run attack when the user uses deposit/stake functions if safeTransferFrom is not called in a single transaction with approval.

In this case, the attacker could use receiver_ as their address and from_ as the victim's address, and the attacker could succeed in depositing the victim's tokens.

Recommendations

We recommend changing _from to msg.sender in depositReserveAssets, depositRewardAssets, and depositRewardAssets. This could prevent an attacker from using other's approval even when the user does not use a single transaction.

Remediation

Cozy Finance acknowledged the risk of front-running when not using a single transaction. Cozy Finance will provide router design that will have a multicall function for building the single transaction.

Cozy Finance fixed depositReserveAssets, depositRewardAssets, and depositRewardAssets for the users who do not use the multicall router.