karanctf - _withdrawfee can be avoided by setting deviationBP as 0

[sherlock-admin]

karanctf

medium

_withdrawfee can be avoided by setting deviationBP as 0

Summary

Deposit and withdraws fee in all cases can be avoided by setting deviationBP as 0 In deposit and withdraw funciton in Dispatcher.sol deviationBP is send as an argument which can be set to 0.

Vulnerability Detail

function deposit(uint256 pid, uint256 amount, uint256 deviationBP) external checkPid(pid) {// @audit why letting user set deviationbp as it can be set to 0 

Impact

No fee will be paid causing loss for protocol

Code Snippet

https://github.com/sherlock-audit/2023-06-real-wagmi/blob/main/concentrator/contracts/Dispatcher.sol#L160 In withdrawFee function

uint256 amount0OutMin = (reserve0 * lpAmount * deviationBP) /
            (_totalSupply * MAX_DEVIATION);
        uint256 amount1OutMin = (reserve1 * lpAmount * deviationBP) /
            (_totalSupply * MAX_DEVIATION);

Tool used

Manual Review

Recommendation

Prevent deviationBP to be set by user.

[ctf-sec]

I agree this finding is invalid after a second look

https://github.com/sherlock-audit/2023-06-real-wagmi/blob/82a234a5c2c1fc1921c63265a9349b71d84675c4/concentrator/contracts/Dispatcher.sol#L160

    function _withdrawFee(
        PoolInfo memory pool,
        uint256 lpAmount,
        uint256 reserve0,
        uint256 reserve1,
        uint256 _totalSupply,
        uint256 deviationBP
    ) private {
        uint256 amount0OutMin = (reserve0 * lpAmount * deviationBP) /
            (_totalSupply * MAX_DEVIATION);
        uint256 amount1OutMin = (reserve1 * lpAmount * deviationBP) /
            (_totalSupply * MAX_DEVIATION);
        (uint256 withdrawnAmount0, uint256 withdrawnAmount1) = IMultipool(pool.multipool).withdraw(
            lpAmount,
            amount0OutMin,
            amount1OutMin
        );
        _pay(pool.token0, address(this), msg.sender, withdrawnAmount0);
        _pay(pool.token1, address(this), msg.sender, withdrawnAmount1);
    }

deviationBP offer a slippage protection because amount0OutMin and amount1OutMin derives from the parameter deviationBP, if the setting deviationBP to 0, that means user decline the slippage protection