ERC4626Transformer Manual Review Findings

ERC-01M: Inexistent Validation of Array Length

Type	Severity	Location
Input Sanitization		ERC4626Transformer.sol:L29, L38, L71, L85

Description:

The linked array input arguments are not validated as possessing a length of 1 which can cause significant issues in integrators of the system as they may "trust" the array input as an actual source of the deposited funds and credit the user inappropriately.

Example:

Copy
68/// @inheritdoc ITransformer
69function transformToDependent(
70  address _dependent,
71  UnderlyingAmount[] calldata _underlying,
72  address _recipient
73) external payable returns (uint256 _amountDependent) {
74  IERC20 _underlyingToken = IERC20(_underlying[0].underlying);
75  uint256 _underlyingAmount = _underlying[0].amount;
76  // We need to take the tokens from the sender, and approve them so that the vault can take it from us
77  _underlyingToken.safeTransferFrom(msg.sender, address(this), _underlyingAmount);
78  _underlyingToken.approve(_dependent, _underlyingAmount);
79  _amountDependent = IERC4626(_dependent).deposit(_underlyingAmount, _recipient);
80}

Recommendation:

We advise the length to be properly validated as 1 in the referenced instances to ensure the codebase can be integrated with at all points of the Mean Finance ecosystem properly.

Alleviation (6ed56b5449ca241fc6be369d44f392f1f5313f93):

Length sanitization was introduced throughout all referenced functions thereby disallowing "spoofing" of amounts that were transformed and alleviating this exhibit in full.

ERC-02M: Improper Approval Methodology

Type	Severity	Location
Standard Conformity		ERC4626Transformer.sol:L78, L102, L106-L108

Description:

The approve methodology applied by the contract performs a direct overwrite of any pre-existing approval with the new one and will cause certain tokens to become in-approvable and thus inoperable by the system such as Tether (USDT), which expects the approval to be zero prior to being overwritten with a new value.

Impact:

Currently, tokens such as Tether (USDT) will fail to function properly under the transformToExpectedDependent and transformToDependent workflows as they utilize an approve system that is incompatible with them.

Example:

Copy
91/// @inheritdoc ITransformer
92function transformToExpectedDependent(
93  address _dependent,
94  uint256 _expectedDependent,
95  address _recipient
96) external payable returns (UnderlyingAmount[] memory) {
97  // Check how much underlying would be needed to mint the vault tokens
98  uint256 _neededUnderlying = IERC4626(_dependent).previewMint(_expectedDependent);
99  // Take the needed underlying tokens from the caller, and approve the vault
100  IERC20 _underlying = IERC20(IERC4626(_dependent).asset());
101  _underlying.safeTransferFrom(msg.sender, address(this), _neededUnderlying);
102  _underlying.approve(_dependent, _neededUnderlying);
103  // Mint the vault tokens
104  uint256 _spentUnderlying = IERC4626(_dependent).mint(_expectedDependent, _recipient);
105  // If some tokens were left unspent, then return to caller
106  if (_spentUnderlying < _neededUnderlying) {
107    _underlying.safeTransfer(msg.sender, _neededUnderlying - _spentUnderlying);
108  }
109  return _toUnderylingAmount(address(_underlying), _spentUnderlying);
110}

Recommendation:

To make sure all types of tokens are accounted for by the system, we advise the approve calls to instead be internalized in a dedicated _approveToken function that will perform an approval of 0 prior to setting the non-zero approval, ensuring that the system is compatible with all types of tokens currently active in the market.

Alleviation (6ed56b5449ca241fc6be369d44f392f1f5313f93):

After discussion with the Mean Finance team, we realized that the first referenced approve instruction is safe as is due to the approval being consumed in full within the deposit EIP-4626 call. As a result, only the second approve instruction needed to be fixed and the Mean Finance team introduced an additional approval of zero statement within the if branch refunding any unspent approved funds. As a result, the exhibit has been alleviated in full.

ERC-03M: Improper payable Trait Definitions

Type	Severity	Location
Language Specific		ERC4626Transformer.sol:L62, L73, L87, L96

Description:

The referenced functions are all declared as payable, however, the contract operates with the EIP-4626 standard which explicitly denotes it only deals with EIP-20 underlying assets thus rendering native transfers to the contract and all contracts that interact with it incorrect.

Impact:

It is currently possible for native funds to be locked in the contract and solely redeemable via administrative processes, an inconvenience that can be avoided via proper programming practices.

Example:

Copy
82/// @inheritdoc ITransformer
83function transformToExpectedUnderlying(
84  address _dependent,
85  UnderlyingAmount[] calldata _expectedUnderlying,
86  address _recipient
87) external payable returns (uint256 _spentDependent) {
88  _spentDependent = IERC4626(_dependent).withdraw(_expectedUnderlying[0].amount, _recipient, msg.sender);
89}

Recommendation:

We advise the payable modifiers from the referenced functions to be omitted as they can lead to loss of funds and do not affect the functionality of other contracts such as Multicall as a delegatecall with zero ether is identical to a call to a non-payable function.

Alleviation (6ed56b5449ca241fc6be369d44f392f1f5313f93):

The Mean Finance team evaluated this exhibit but opted not to apply a remediation for it in the current version of the codebase as they deem it a non-issue. As a result, we consider the exhibit acknowledged.

ERC-04M: Inexistent Enforcement of Slippage Checks

Type	Severity	Location
Logical Fault		ERC4626Transformer.sol:L64, L79, L88, L104

Description:

The EIP-4626 standard is naturally prone to slippage checks as it transforms an asset to another, however, the functions that utilize the vault's conversion methods do not account for this fact and thus perform the swaps blindly, allowing arbitrage attacks to be performed when large transforms are identified.

Impact:

In the current code's state, the system is fully susceptible to arbitrage attacks (a.k.a. as sandwich attacks) that inflate and deflate the value of a particular asset / vault share prior to and after the execution of a transaction that performs a significant transformation. This is especially exploitable via the usage of MEV bribes.

Example:

Copy
82/// @inheritdoc ITransformer
83function transformToExpectedUnderlying(
84  address _dependent,
85  UnderlyingAmount[] calldata _expectedUnderlying,
86  address _recipient
87) external payable returns (uint256 _spentDependent) {
88  _spentDependent = IERC4626(_dependent).withdraw(_expectedUnderlying[0].amount, _recipient, msg.sender);
89}

Recommendation:

We advise the system to enforce a slippage check either at the BaseTransformer level (by defining internal functions that derivative implementations override) or at the TransformerRegistry level by adapting the external ABI to account for additional argument(s).

Alleviation (6ed56b5449ca241fc6be369d44f392f1f5313f93):

Slippage checks were introduced throughout all referenced functions ensuring that the EIP-4626 interactions are secured against sandwich attacks and similar slippage-related vectors if configured properly.