Summary
PerpetualVault.sol#_cancelFlow() function transfers collateral to owner of depositInfo.
But this can be reverted by some reasons, for example, usdc blacklist.
Then, the protocol can not be recovered by keeper.
Vulnerability Details
PerpetualVault.sol#_transferToken() function is as follows.
function _transferToken(uint256 depositId, uint256 amount) internal {
uint256 fee;
if (amount > depositInfo[depositId].amount) {
fee = (amount - depositInfo[depositId].amount) * governanceFee / BASIS_POINTS_DIVISOR;
if (fee > 0) {
collateralToken.safeTransfer(treasury, fee);
}
}
@> try collateralToken.transfer(depositInfo[depositId].recipient, amount - fee) {}
catch {
collateralToken.transfer(treasury, amount - fee);
emit TokenTranferFailed(depositInfo[depositId].recipient, amount - fee);
}
totalDepositAmount -= depositInfo[depositId].amount;
emit GovernanceFeeCollected(address(collateralToken), fee);
}
As we can see above, try-catch block is used in _transferToken().
But _cancelFlow() function missed it.
function _cancelFlow() internal {
if (flow == FLOW.DEPOSIT) {
uint256 depositId = counter;
@> collateralToken.safeTransfer(depositInfo[depositId].owner, depositInfo[depositId].amount);
totalDepositAmount = totalDepositAmount - depositInfo[depositId].amount;
EnumerableSet.remove(userDeposits[depositInfo[depositId].owner], depositId);
try IGmxProxy(gmxProxy).refundExecutionFee(
depositInfo[counter].owner,
depositInfo[counter].executionFee
) {} catch {}
delete depositInfo[depositId];
} else if (flow == FLOW.WITHDRAW) {
try IGmxProxy(gmxProxy).refundExecutionFee(
depositInfo[counter].owner,
depositInfo[counter].executionFee
) {} catch {}
}
flow = FLOW.LIQUIDATION;
nextAction.selector = NextActionSelector.FINALIZE;
}
Impact
The protocol can not be recovered by keeper because keeper can not cancel flow in some cases.
Of course, owner can recover protocol through setVaultState() but depositor's fund is freezed to protocol.
The owner can not transfer it to treasury.
Tools Used
Manual Review
Recommendations
Modify PerpetualVault.sol#_cancelFlow() function as follows.
function _cancelFlow() internal {
if (flow == FLOW.DEPOSIT) {
uint256 depositId = counter;
-- collateralToken.safeTransfer(depositInfo[depositId].owner, depositInfo[depositId].amount);
++ try collateralToken.safeTransfer(depositInfo[depositId].owner, depositInfo[depositId].amount) {}
++ catch {
++ collateralToken.safeTransfer(treasury, depositInfo[depositId].amount);
++ emit TokenTranferFailed(depositInfo[depositId].owner, depositInfo[depositId].amount);
++ }
totalDepositAmount = totalDepositAmount - depositInfo[depositId].amount;
EnumerableSet.remove(userDeposits[depositInfo[depositId].owner], depositId);
try IGmxProxy(gmxProxy).refundExecutionFee(
depositInfo[counter].owner,
depositInfo[counter].executionFee
) {} catch {}
delete depositInfo[depositId];
} else if (flow == FLOW.WITHDRAW) {
try IGmxProxy(gmxProxy).refundExecutionFee(
depositInfo[counter].owner,
depositInfo[counter].executionFee
) {} catch {}
}
flow = FLOW.LIQUIDATION;
nextAction.selector = NextActionSelector.FINALIZE;
}
