code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Market.sol#L149 https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Market.sol#L161 https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Market.sol#L172 https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Market.sol#L183 https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Market.sol#L194

Vulnerability details

Impact

Governance of Market.sol can call following function at anytime. This is not ideal since they can call this function for their own benefits. For example they can change liquidationFactorBps to gain more liquidationFee. They can change collateralFactorBps to make user borrow less or to liquidate them.

setCollateralFactorBps setLiquidationFactorBps setReplenismentIncentiveBps setLiquidationIncentiveBps setLiquidationFeeBps

Proof of Concept

No implementation are done to avoid malicious governance to change these important state variables at any time they want.

    function setCollateralFactorBps(uint _collateralFactorBps) public onlyGov {
        require(_collateralFactorBps < 10000, "Invalid collateral factor");
        collateralFactorBps = _collateralFactorBps;
    }

    function setLiquidationFactorBps(uint _liquidationFactorBps) public onlyGov {
        require(_liquidationFactorBps > 0 && _liquidationFactorBps <= 10000, "Invalid liquidation factor");
        liquidationFactorBps = _liquidationFactorBps;
    }

    function setReplenismentIncentiveBps(uint _replenishmentIncentiveBps) public onlyGov {
        require(_replenishmentIncentiveBps > 0 && _replenishmentIncentiveBps < 10000, "Invalid replenishment incentive");
        replenishmentIncentiveBps = _replenishmentIncentiveBps;
    }

    function setLiquidationIncentiveBps(uint _liquidationIncentiveBps) public onlyGov {
        require(_liquidationIncentiveBps > 0 && _liquidationIncentiveBps + liquidationFeeBps < 10000, "Invalid liquidation incentive");
        liquidationIncentiveBps = _liquidationIncentiveBps;
    }

    function setLiquidationFeeBps(uint _liquidationFeeBps) public onlyGov {
        require(_liquidationFeeBps > 0 && _liquidationFeeBps + liquidationIncentiveBps < 10000, "Invalid liquidation fee");
        liquidationFeeBps = _liquidationFeeBps;
    }

Tools Used

Manual Analysis

Recommended Mitigation Steps

Add timelock implementation so that user can know the change before it gets executed. Reference: https://blog.chain.link/timelock-smart-contracts/

neumoxx commented 1 year ago

Should be probably QA.

c4-judge commented 1 year ago

0xean marked the issue as unsatisfactory: Overinflated severity

code-423n4 / 2022-10-inverse-findings