Olympus DAO contest - ak1's results

Lines of code

https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/modules/PRICE.sol#L161-L173

Vulnerability details

Impact

Inconsistency in determining the getCurrentPrice for ohmEthPrice and reserveEthPrice.

Proof of Concept

        (, int256 ohmEthPriceInt, , uint256 updatedAt, ) = _ohmEthPriceFeed.latestRoundData();
        // Use a multiple of observation frequency to determine what is too old to use.
        // Price feeds will not provide an updated answer if the data doesn't change much.
        // This would be similar to if the feed just stopped updating; therefore, we need a cutoff.
        if (updatedAt < block.timestamp - 3 * uint256(observationFrequency))
            revert Price_BadFeed(address(_ohmEthPriceFeed));
        ohmEthPrice = uint256(ohmEthPriceInt);


        int256 reserveEthPriceInt;
        (, reserveEthPriceInt, , updatedAt, ) = _reserveEthPriceFeed.latestRoundData();
        if (updatedAt < block.timestamp - uint256(observationFrequency))
            revert Price_BadFeed(address(_reserveEthPriceFeed));
        reserveEthPrice = uint256(reserveEthPriceInt);

In getCurrentPrice(), the ohmEthPrice is validated at multiple of observationFrequency

if (updatedAt < block.timestamp - 3 * uint256(observationFrequency)) revert Price_BadFeed(address(_ohmEthPriceFeed));

But for reserveEthPriceInt , it is not as same above. if (updatedAt < block.timestamp - uint256(observationFrequency)) revert Price_BadFeed(address(_reserveEthPriceFeed));

Tools Used

Manual code review, VS code

Recommended Mitigation Steps

Use multiple of frequency for reserveEthPriceInt also, if (updatedAt < block.timestamp - 3 * uint256(observationFrequency)) revert Price_BadFeed(address(_reserveEthPriceFeed));

Lines of code

https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/BondCallback.sol#L48 https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/Governance.sol#L61 https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/Heart.sol#L69 https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/Operator.sol#L154 https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/PriceConfig.sol#L18 https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/TreasuryCustodian.sol#L27 https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/VoterRegistration.sol#L19

Vulnerability details

Impact

Any one from outside can call the configureDependencies.All are external without access restriction

configureDependencies in BondCallback has ohm.safeApprove(address(MINTR), type(uint256).max);- this could be serious issue to the protocol. Also,configureDependenciesin Operatorhasohm.safeApprove(address(MINTR), type(uint256).max);` - this also could be serious issue to the protocol. Unexpected/unstable protocol behavior.

Proof of Concept

Refer the below lines of codes in all the policy contracts. https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/BondCallback.sol#L48 https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/Governance.sol#L61 https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/Heart.sol#L69 https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/Operator.sol#L154 https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/PriceConfig.sol#L18 https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/TreasuryCustodian.sol#L27 https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/VoterRegistration.sol#L19

Tools Used

VS code, Manual code review. Confirmed with sponsor - fully#0001

Recommended Mitigation Steps

Add access modifier onlyKernel so that only the kernel can configure the policies.