DeFiFoundrySolidity
16,653 OP
View results
Submission Details
Severity: medium
Invalid

Misaligned Documentation and Implementation in _freeFunds Function—Potential for User Loss

Summary

The _freeFunds function is designed to release a specified amount of assets during withdraw or redeem operations in the contracts of StrategyMainnet.sol, StrategyArb.solandStrategyOp.sol. The _freeFunds function's comment states: "Any difference between _amountand what is actually freed will be counted as a loss and passed on to the withdrawer. This means care should be taken in times of illiquidity. may be better to revert if withdrawals are simply illiquid so as not to realize incorrect losses." However, the implementation does not follow this approach. Instead, when_amount > totalAvailable`, the function releases all available funds, and the shortfall is treated as a loss borne by the withdrawer. This inconsistency between the documented intent and the actual behavior introduces confusion and could lead to unintended consequences and introduces potential risks, especially during times of illiquidity.

Vulnerability Details

src/StrategyMainnet.sol:_freeFunds#L138-L139
src/StrategyArb.sol:_freeFunds#L114-L115
src/StrategyOp.sol:_freeFunds#L127-L128

* Any difference between `_amount` and what is actually freed will be
* counted as a loss and passed on to the withdrawer. This means
* care should be taken in times of illiquidity. It may be better to revert
* if withdraws are simply illiquid so not to realize incorrect losses.
function _freeFunds(uint256 _amount) internal override {
// @audit difference between comments and implementation
uint256 totalAvailable = transmuter.getUnexchangedBalance(address(this));
if (_amount > totalAvailable) {
// Release all available funds if the requested amount exceeds availability
// @audit loss passed on to the withdrawer
transmuter.withdraw(totalAvailable, address(this));
} else {
// Release only the requested amount if sufficient funds are available
transmuter.withdraw(_amount, address(this));
}
}

The comment suggests the function should revert in times of illiquidity, protecting users from incorrect losses. However, the implementation allows withdrawals, passing the shortfall as a loss to the user, contrary to the documented intent.

The current implementation unfairly shifts the burden of illiquidity from the protocol to the users, which may not align with user expectations based on the comments.

When _amount > totalAvailable, the difference is treated as a loss and passed to the withdrawer. This behavior could result in incorrect loss realization during temporary liquidity shortages. Withdrawers may suffer losses due to illiquidity, even if the shortfall is temporary or recoverable.

Additionally, the realized losses may not align with the actual performance of the underlying assets, potentially misleading stakeholders.

Impact

  • Users withdrawing during times of illiquidity may face significant losses.

  • The realized losses may not align with the actual performance of the underlying assets

  • Misunderstood behavior may lead to improper integration with other systems, increasing the risk of exploits.

Tools Used

Manual Review

Recommendations

Please consider to add liquidity validation to ensure the function reverts when _amount > totalAvailable instead of treating the shortfall as a loss:

function _freeFunds(uint256 _amount) internal override {
+ require(_amount > 0, "Invalid amount");
uint256 totalAvailable = transmuter.getUnexchangedBalance(address(this));
+ if (totalAvailable < _amount && !isEmergency) {
+ revert InsufficientLiquidity();
+ }
+ uint256 amountToWithdraw = _amount > totalAvailable ?
+ totalAvailable : _amount;
+ emit FundsFreed(amountToWithdraw, _amount);
transmuter.withdraw(amountToWithdraw, address(this));
}}
Updates

Appeal created

inallhonesty Lead Judge 8 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.