Description:
The execution of WithdrawETH action is secured in a way that it checks whether the previous action was Borrow or Withdraw action and the WithdrawETH receiver is the same as the owner of assets from previous action.
There are two issues with this construction:
There is a typo in the expression. It should be actions[i-1] != Action.Borrow instead of actions[i] != Action.Borrow
The mentioned mechanism can be bypassed as presented in the Attack Scenario.
Attack scenario
The attacker prepares the following bundle:
a. PermitWithdraw of leaked victim’s permit for a vault with WETH ascollateral,
b. Withdraw of victim’s collateral to router,
c. Withdraw of attacker’s collateral to router (this action bypasses the protection mechanism),
d. WithdrawETH of all Ether gathered on the router.
Recommendation:
This vulnerability can be avoided by removing the mentioned protection mechanism and implementing the one from “Insecure implementation of approvals and permits” as the root cause is the same.
0xdcota
commented
1 year ago
Where: https://github.com/Fujicracy/fuji-v2/blob/bffa427797ad8d6df63671868ee8823574e044ca/packages/protocol/src/abstracts/BaseRouter.sol#L210-L214
Description: The execution of WithdrawETH action is secured in a way that it checks whether the previous action was Borrow or Withdraw action and the WithdrawETH receiver is the same as the owner of assets from previous action. There are two issues with this construction:
Attack scenario
Recommendation: This vulnerability can be avoided by removing the mentioned protection mechanism and implementing the one from “Insecure implementation of approvals and permits” as the root cause is the same.