Olas - imare's results

Lines of code

https://github.com/code-423n4/2023-12-autonolas/blob/2a095eb1f8359be349d23af67089795fb0be4ed1/tokenomics/contracts/Tokenomics.sol#L301-L303

Vulnerability details

'Tokenomics' contract is an integral part ot he autonolas protocol. It is used as the central part which all other contract in the protocol call. But it can be initialized in a state where core functionality is hindered.

Even if the protocol tries to repair the Tokenomics initialization later by calling changeTokenomicsParameters function, it will still not work correctly.

Impact

The Tokenomics contract can be initialized in a faulty state. This state cannot be reversed. If it happens the protocol stops working correctly.

Proof of Concept

If we initialize the epochLen parameter to a value exactly to a year value (it can also be some seconds before a full year) the protocol will stop from working correctly.

https://github.com/code-423n4/2023-12-autonolas/blob/2a095eb1f8359be349d23af67089795fb0be4ed1/tokenomics/contracts/Tokenomics.sol#L301-L303

The following POC will show that the Tokenomics#initializeTokenomics can be called with a epochLean set to a year:

// this test is added to the Treasurt.t.sol test file: // NOTE: inside the checpoint() function I removed the checking for the presence of the proxy implementation becouse we are not testing it:

    function checkpoint() external returns (bool) {
        ...
        // Check if there is any address in the PROXY_TOKENOMICS address slot
-       if (implementation == address(0)) {
-            revert DelegatecallOnly();
-       }
        // Check if there is any address in the PROXY_TOKENOMICS address slot
+       // if (implementation == address(0)) {
+       //     revert DelegatecallOnly();
+       // }
        ...

function test_TokenomicsInit () public {
        vm.warp(block.timestamp);
        Tokenomics tokenomics2 = new Tokenomics();
        uint256 epLen =  1 days * 364;
        tokenomics2.initializeTokenomics(address(olas), address(deployer), address(deployer), address(deployer),
            address(deployer), epLen, address(deployer), address(deployer), address(deployer), address(0));

        vm.warp(block.timestamp + epLen + 1);
        bool canCheckPoint = tokenomics2.checkpoint();
        assertTrue(canCheckPoint, "Checkpoint #1 should return true");

        // @-> init to full year .. initialization should fail but it does not
        Tokenomics tokenomics3 = new Tokenomics();
        epLen =  1 days * 365;
        tokenomics3.initializeTokenomics(address(olas), address(deployer), address(deployer), address(deployer),
            address(deployer), epLen, address(deployer), address(deployer), address(deployer), address(0));

        canCheckPoint = tokenomics3.checkpoint();
        assertTrue(canCheckPoint, "Checkpoint #2 should return true");

        // @ -> this should definitely return true 
        vm.warp(block.timestamp + epLen + 1);
        canCheckPoint = tokenomics3.checkpoint();
        assertTrue(canCheckPoint, "Checkpoint #3 should return true");
    }

If we run the test we get that initialization in our two Tokenomics instances goes trough but the second one which have epochLen set to a year value always fails on calling checkpoint:

[FAIL. Reason: assertion failed] test_TokenomicsInit() (gas: 9593773)
Logs:
  Error: Checkpoint #2 should return true
  Error: Assertion Failed
  Error: Checkpoint #3 should return true
  Error: Assertion Failed

The following test will show that once in this faulty state we cannot update to a correct one by changing the epochLen parameter :

function test_TokenomicsFaultInitCannotReverse () public {
        // set faulty  instance
        Tokenomics tokenomics3 = new Tokenomics();
        uint256 epLen =  1 days * 365;
        tokenomics3.initializeTokenomics(address(olas), address(deployer), address(deployer), address(deployer),
            address(deployer), epLen, address(deployer), address(deployer), address(deployer), address(0));

        uint256 correctValue = 30 days;
        tokenomics3.changeTokenomicsParameters(0, 0, 0, correctValue, 0);

        assertEq(tokenomics3.epochLen(), correctValue);
    }

The test should pass but it fails :

[FAIL. Reason: assertion failed] test_TokenomicsFaultInitCannotReverse() (gas: 4756080)
Logs:
  Error: a == b not satisfied [uint]
        Left: 31536000
       Right: 2592000

If we set the epochLen to a year in the javascript tests there will also display errors:

The checkpoint() will now always return false and becouse of that other part of the protocol will not work correctly:

https://github.com/code-423n4/2023-12-autonolas/blob/2a095eb1f8359be349d23af67089795fb0be4ed1/tokenomics/test/Tokenomics.js#L26 to

-   const epochLen = oneMonth;
+   const epochLen = oneYear;

If we run the test for Tokenomics than 5 tests will fail:

npx hardhat test  --grep "Tokenomics"

  1) Tokenomics
       Initialization
         Changing tokenomics parameters:
  2) Tokenomics
       Tokenomics calculation
         Checkpoint with inability to re-balance treasury rewards:
  3) Tokenomics
       Incentives
         Calculate incentives:
  4) Tokenomics
       Incentives
         Changing maxBond values:
  5) Tokenomics
       Time sensitive tests
         Get to the epoch before the end of the OLAS year and try to change maxBond or epochLen:

In Dispenser.js if we change the epochLength to a year a try to test the claiming of incentives all test will fail :

  Dispenser
    Get incentives

      1) Claim incentives for unit owners

      2) Claim incentives for unit owners when donator and service owner are different accounts

      3) Claim incentives for unit owners for more than one epoch

      4) Claim incentives for unit owners: component and agent reward fractions are zero

      5) Claim incentives for unit owners: component incentive fractions are zero

      6) Claim incentives for unit owners: component and agent top-ups fractions are zero

      7) Claim incentives for unit owners: incentives are not zero at first, but then zero towards end of epoch

      8) Claim incentives for unit owners: all incentive fractions are zero

Tools Used

Manual review

Recommended Mitigation Steps

Make some leeway when setting the epochLen state variable on initialization and in the changeTokenomicsParameters setter :

...
+    uint256 public constant LEEWAY = 7 days;
...
    function initializeTokenomics(
        ...
        // Check that the epoch length is not bigger than one year
-       if (uint32(_epochLen) > ONE_YEAR) {
+       if (uint32(_epochLen) > (ONE_YEAR-LEEWAY)) {
            revert Overflow(_epochLen, ONE_YEAR);
        }
        ...

    function changeTokenomicsParameters(
    ...
            // Check for the epochLen value to change
-       if (uint32(_epochLen) >= MIN_EPOCH_LENGTH && uint32(_epochLen) <= ONE_YEAR) {
+        if (uint32(_epochLen) >= MIN_EPOCH_LENGTH && uint32(_epochLen) <= (ONE_YEAR-LEEWAY)) {
            nextEpochLen = uint32(_epochLen);
        } else {
            _epochLen = epochLen;
        }
    ...

Assessed type

Error