[WP-M3] `TurboRouter.sol#createSafeAndDeposit*()` CreateSafeAndDeposit combo methods won't work as an allowance cannot be granted to a newly created Safe for deposit

[code423n4]

Lines of code

https://github.com/fei-protocol/tribe-turbo/blob/5e1c5d9b49dc557c84f07afabbba2ba4e08e9cc6/src/TurboRouter.sol#L49-L72

Vulnerability details

The TurboRouter.sol#deposit() function can be used in a multicall() together with approve() and pullToken() from PeripheryPayments to pull tokens from msg.sender and grant allowance for the ERC4626 Safe to call asset.safeTransferFrom() with the msg.sender being the router's address.

However, that would not work for createSafeAndDeposit() and createSafeAndDepositAndBoost(), because a newly created Safe address can only be known at runtime.

https://github.com/fei-protocol/tribe-turbo/blob/5e1c5d9b49dc557c84f07afabbba2ba4e08e9cc6/src/TurboRouter.sol#L49-L72

function createSafeAndDeposit(ERC20 underlying, address to, uint256 amount, uint256 minSharesOut) external {
    (TurboSafe safe, ) = master.createSafe(underlying);

    super.deposit(IERC4626(address(safe)), to, amount, minSharesOut);

    safe.setOwner(msg.sender);
}

function createSafeAndDepositAndBoost(
    ERC20 underlying, 
    address to, 
    uint256 amount, 
    uint256 minSharesOut, 
    ERC4626 boostedVault, 
    uint256 boostedFeiAmount
) public {
    (TurboSafe safe, ) = master.createSafe(underlying);

    super.deposit(IERC4626(address(safe)), to, amount, minSharesOut);

    safe.boost(boostedVault, boostedFeiAmount);

    safe.setOwner(msg.sender);
}

Recommendation

Change to:

function createSafeAndDeposit(ERC20 underlying, address to, uint256 amount, uint256 minSharesOut) external {
    (TurboSafe safe, ) = master.createSafe(underlying);

    underlying.safeTransferFrom(msg.sender, address(this), amount);
    underlying.safeApprove(address(safe), amount);
    super.deposit(IERC4626(address(safe)), to, amount, minSharesOut);

    safe.setOwner(msg.sender);
}

function createSafeAndDepositAndBoost(
    ERC20 underlying, 
    address to, 
    uint256 amount, 
    uint256 minSharesOut, 
    ERC4626 boostedVault, 
    uint256 boostedFeiAmount
) public {
    (TurboSafe safe, ) = master.createSafe(underlying);

    underlying.safeTransferFrom(msg.sender, address(this), amount);
    underlying.safeApprove(address(safe), amount);
    super.deposit(IERC4626(address(safe)), to, amount, minSharesOut);

    safe.boost(boostedVault, boostedFeiAmount);

    safe.setOwner(msg.sender);
}

[Joeysantoro]

Router uses Multicall and PeripheryPayments which can be combined to achieve the desired behaviors

[Joeysantoro]

Duplicate of https://github.com/code-423n4/2022-02-tribe-turbo-findings/issues/16

[GalloDaSballo]

Duplicate of #16