code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/TurboMaster.sol#L161

Vulnerability details

Impact

A user can't create a safe because of the requiresAuth modifier in createSafe(). Neither directly through the TurboMaster contract nor through the router.

Proof of Concept

Here's the test file I used to confirm it. I had to modify the contracts a little bit to satisfy the dependencies so you can't just drop it into the codebase. But that should be fairly easy for you to replicate. The issue can also be verified by just looking at the createSafe() function.

// SPDX-License-Identifier: AGPL-3.0-only
pragma solidity 0.8.10;

import {ERC20} from "solmate/tokens/ERC20.sol";
import {Authority} from "solmate/auth/Auth.sol";
import {DSTestPlus} from "solmate/test/utils/DSTestPlus.sol";
import {MockERC20} from "solmate/test/utils/mocks/MockERC20.sol";
import {WETH} from "solmate/tokens/WETH.sol";

import {MockCToken} from "./mocks/MockCToken.sol";
import {MockPriceFeed} from "./mocks/MockPriceFeed.sol";
import {MockFuseAdmin} from "./mocks/MockFuseAdmin.sol";
import {MockComptroller} from "./mocks/MockComptroller.sol";

import {TurboClerk} from "../modules/TurboClerk.sol";
import {TurboBooster} from "../modules/TurboBooster.sol";

import {TurboSafe} from "../TurboSafe.sol";
import {TurboMaster} from "../TurboMaster.sol";
import {TurboRouter} from "../TurboRouter.sol";

import {IERC4626, ERC4626RouterBase, PeripheryPayments} from "ERC4626/ERC4626RouterBase.sol";
import "./User.sol";

contract User {

  TurboMaster master;

  constructor(TurboMaster _master) {
    master = _master;
  }

  function createSafe(ERC20 asset) public {
    master.createSafe(asset);
  }
}

contract TurboRouterTest is DSTestPlus {

    MockFuseAdmin fuseAdmin;

    MockComptroller comptroller;

    MockERC20 fei;

    MockERC20 asset;

    MockCToken mockCToken;

    TurboMaster master;
    TurboRouter router;
    User user;

    WETH weth = new WETH();
    function setUp() public {
        fei = new MockERC20("Fei USD", "FEI", 18);

        asset = new MockERC20("Mock Token", "MOCK", 18);

        fuseAdmin = new MockFuseAdmin();

        comptroller = new MockComptroller(address(fuseAdmin), new MockPriceFeed());

        master = new TurboMaster(comptroller, fei, address(this), Authority(address(0)));

        MockCToken assetCToken = new MockCToken(asset);

        comptroller.mapUnderlyingToCToken(asset, assetCToken);

        assertEq(master.getAllSafes().length, 1);

        user = new User(master);

        router = new TurboRouter(master, "router2", weth);
    }

    function testUserCreate() public {
       user.createSafe(asset);
    }
    function testCreateSafe() public {
      router.createSafe(asset);
    }
}

Both tests revert with the following msg: UNAUTHORIZED

Tools Used

none

Recommended Mitigation Steps

remove the modifier since it's a basic user facing function.

Joeysantoro commented 2 years ago

Safe creation authentication is managed by Tribe governance. This is not an issue as governance will configure the turbo launch in a desirable manner.

GalloDaSballo commented 2 years ago

While I think there's legitimacy to the idea of making the creation of a safe trustless, I don't see any vulnerability here.

code-423n4 / 2022-02-tribe-turbo-findings