Both createSafeAndDeposit function and createSafeAndDepositAndBoost would revert on every call.
Proof of Concept
Both functions suffers from the same mistake so I'll detailed only on createSafeAndDeposit (link 1).
First the function calls master.createSafe(underlying); to create a safe.
the msg.sender in the master.createSafe() contex is the TurboRouter contract, so the owner of the safe that was created is the TurboRouter contract.
Now we call super.deposit(IERC4626(address(safe)), to, amount, minSharesOut); (implemention of this deposit funciton is in link 2).
In there we can see the next line:
if ((sharesOut = vault.deposit(amount, to)) < minSharesOut),
let's jump to the implementation of vault.deposit(amount, to)) (link 3).
The msg.sender in this contex is again the TurboRouter contract.
Which means in this line: asset.safeTransferFrom(msg.sender, address(this), amount); we trasfer from the TurboRouter contract to the safe, so the TurboRouter contract has to give allowence for the safe contract, but it does not happen anywhere, so the trasferFrom call will revert.
Tools Used
read the code
Recommended Mitigation Steps
Consider give approvemence from the TurboRouter contract to the created safe.
Lines of code
https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/TurboRouter.sol#L49 https://github.com/fei-protocol/ERC4626/blob/5b786fe0317f65f5b716f577c28092fa349c4903/src/ERC4626RouterBase.sol#L29 https://github.com/Rari-Capital/solmate/blob/main/src/mixins/ERC4626.sol#L47
Vulnerability details
Impact
Both
createSafeAndDeposit
function andcreateSafeAndDepositAndBoost
would revert on every call.Proof of Concept
Both functions suffers from the same mistake so I'll detailed only on
createSafeAndDeposit
(link 1). First the function callsmaster.createSafe(underlying);
to create a safe. themsg.sender
in themaster.createSafe()
contex is theTurboRouter
contract, so the owner of the safe that was created is theTurboRouter
contract. Now we callsuper.deposit(IERC4626(address(safe)), to, amount, minSharesOut);
(implemention of this deposit funciton is in link 2). In there we can see the next line:if ((sharesOut = vault.deposit(amount, to)) < minSharesOut)
, let's jump to the implementation ofvault.deposit(amount, to))
(link 3). The msg.sender in this contex is again theTurboRouter
contract. Which means in this line:asset.safeTransferFrom(msg.sender, address(this), amount);
we trasfer from theTurboRouter
contract to thesafe
, so theTurboRouter
contract has to give allowence for thesafe
contract, but it does not happen anywhere, so the trasferFrom call will revert.Tools Used
read the code
Recommended Mitigation Steps
Consider give approvemence from the
TurboRouter
contract to the created safe.