When we update the old IdleStrategy, there may be some USDC tokens in that.
Opened this issue · 1 comments
Github username: --
Twitter username: --
Submission hash (on-chain): 0xcfe892ae77881976899587709036d34932275abb12a0a6076b9c9cfdd5bb4558
Severity: low
Description:
Description
An elevated user
has the ability to update the IdleStrategy
in the OrigamiIdleStrategyManager
.
If there is an old IdleStrategy
, we enforce approval of 0
for the old one.
However, we do not check whether there are any allocated USDC
tokens in it.
Attack Scenario
The IdleStrategyManager
has previously interacted with the old IdleStrategy
, O_I
, and some USDC
tokens were allocated to it.
Now, we are preparing to update to the new IdleStrategy
, N_I
.
The following function is invoked:
function setIdleStrategy(address _idleStrategy) external override onlyElevatedAccess {
if (_idleStrategy == address(0)) revert CommonEventsAndErrors.InvalidAddress(_idleStrategy);
address _oldStrategy = address(idleStrategy);
if (_oldStrategy != address(0)) {
asset.forceApprove(_oldStrategy, 0);
}
asset.forceApprove(_idleStrategy, type(uint256).max);
idleStrategy = IOrigamiIdleStrategy(_idleStrategy);
emit IdleStrategySet(_idleStrategy);
}
There is no verification conducted to confirm if there are any allocated USDC
tokens in the O_I
.
Additional actions are required to utilize those USDC
tokens.
Attachments
- Proof of Concept (PoC) File
- Revised Code File (Optional)
Implement functionality to withdraw allocated USDC
tokens from the old IdleStrategy
.
function setIdleStrategy(address _idleStrategy) external override onlyElevatedAccess {
if (_idleStrategy == address(0)) revert CommonEventsAndErrors.InvalidAddress(_idleStrategy);
address _oldStrategy = address(idleStrategy);
if (_oldStrategy != address(0)) {
asset.forceApprove(_oldStrategy, 0);
+ uint256 amount = IOrigamiIdleStrategy(_oldStrategy).availableToWithdraw();
+ if (amount != 0) {
+ IOrigamiIdleStrategy(_oldStrategy).withdraw(amount, address(this));
+ }
}
asset.forceApprove(_idleStrategy, type(uint256).max);
idleStrategy = IOrigamiIdleStrategy(_idleStrategy);
emit IdleStrategySet(_idleStrategy);
}
Thanks for raising
There is no verification conducted to confirm if there are any allocated USDC tokens in the O_I.
This is intentional. If the old idle strategy is compromised we want an immediate way to cut over ASAP. If the check was there we would never be able to cut it over.