code-423n4 / 2023-12-autonolas-findings

3 stars 3 forks source link

[M2] DrainServicesSlashedFunds has not check for received funds #442

Closed c4-bot-10 closed 8 months ago

c4-bot-10 commented 9 months ago

Lines of code

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

Vulnerability details

Impact

​ Loss of funds.

Analysis of the vulnerability

The function to drain funds from Service Registry is not checking that is receiving the correct amount from ServiceRegistry.

         /// @dev Drains slashed funds from the service registry.
    /// @return amount Drained amount.
    /// #if_succeeds {:msg "correct update total eth balance"} address(this).balance == old(address(this).balance) + amount;
    /// #if_succeeds {:msg "conservation law"} ETHFromServices + ETHOwned == old(ETHFromServices + ETHOwned) + amount;
    ///if_succeeds {:msg "slashed funds check"} IServiceRegistry(ITokenomics(tokenomics).serviceRegistry()).slashedFunds() >= minAcceptedETH
    /// ==> old(IServiceRegistry(ITokenomics(tokenomics).serviceRegistry()).slashedFunds()) == amount;

     function drainServiceSlashedFunds() external returns (uint256 amount) {
        // Check for the contract ownership
        if (msg.sender != owner) {
            revert OwnerOnly(msg.sender, owner);
        }

        // Get the service registry contract address
        address serviceRegistry = ITokenomics(tokenomics).serviceRegistry();

        // Check if the amount of slashed funds are at least the minimum required amount to receive by the Treasury
        uint256 slashedFunds = IServiceRegistry(serviceRegistry).slashedFunds();
        if (slashedFunds < minAcceptedETH) {
            revert LowerThan(slashedFunds, minAcceptedETH);
        }

        // Call the service registry drain function
        amount = IServiceRegistry(serviceRegistry).drain();
    }

If there's a bug in the updateable serviceRegistry, funds could be sent elsewhere.

Recommendation

Add at the beginning of the function.

uint balanceBefore = address(this).balance;

And at the end

uint balanceAfter = address(this).balance; require(balanceBefore + amount = balanceAfter, "Balance not propertly updated")

Assessed type

ETH-Transfer

c4-pre-sort commented 9 months ago

alex-ppg marked the issue as insufficient quality report

c4-judge commented 8 months ago

dmvt marked the issue as unsatisfactory: Insufficient quality