082.md

Orbiting Sangria Porpoise

Medium

Missing validation in setSellFee() can cause sell_fee_withPID to never rebase

Description

setSellFee() has the following validation missing. Owner should not be allowed to set sell_fee less than sell_fee_minimum and needs to be explicitly checked, else a situation could arise where rebase of sell_fee_withPID is stuck:

    function setSellFee(uint _fee) external onlyOwner {
        require(_fee <= 1 ether, "fee too high");
+       require(_fee > sell_fee_minimum, "fee too low");
        sell_fee = _fee;

        // careful
        // changing sell fee will reset sell_fee scaling
        sell_fee_withPID = sell_fee;
        lastBlockTime_sell_fee = block.timestamp;
        //sell_fee_update_blocknumber = block.number;

        emit SellFeeUpdated(_fee);
    }

Impact

If sell_fee is set below sell_fee_minimum, the following can happen: - Assume sell_fee_minimum = 0.5 ether (default value). - Owner sets sell_fee = 0.3 ether due to the missing aforementioned check. The function sets sell_fee_withPID too to 0.3 ether. - Inside getSellFeeScaling(), currentLiquidCF < cf_liquid_severe and we need to enter the debase branch. - The code takes care of clipping lastSellFee to sell_fee_minimum. This results in sell_fee_withPID to be updated to 0.5 ether. The value of sell_fee is still at 0.3 ether. - Next time if currentLiquidCF >= cf_liquid_severe and we need to rebase, the conditional check here of if (sell_fee_withPID < sell_fee) is never satisfied and rebase never happens.

The only remedy now is for the owner to call setSellFee() again with a correct value of _fee so that it resets scaling.

From a financial standpoint, this means that users would be stuck getting less LST tokens than they should upon selling Numa.

Mitigation

    function setSellFee(uint _fee) external onlyOwner {
        require(_fee <= 1 ether, "fee too high");
+       require(_fee > sell_fee_minimum, "fee too low");
        sell_fee = _fee;

        // careful
        // changing sell fee will reset sell_fee scaling
        sell_fee_withPID = sell_fee;
        lastBlockTime_sell_fee = block.timestamp;
        //sell_fee_update_blocknumber = block.number;

        emit SellFeeUpdated(_fee);
    }