Summary
PriorityPool is PausableUpgradeable. However, during deposit and withdrawcalls, the whenNotPaused modifier is not implemented making the contract depositable and withdrawable during a possible incident.
Vulnerability Details
Here is the deposit function:
Contract: PriorityPool.sol
254: function deposit(uint256 _amount, bool _shouldQueue, bytes[] calldata _data) external {
255: if (_amount == 0) revert InvalidAmount();
256: token.safeTransferFrom(msg.sender, address(this), _amount);
257: _deposit(msg.sender, _amount, _shouldQueue, _data);
258: }
followed by _deposit call internally:
Contract: PriorityPool.sol
601: function _deposit(
602: address _account,
603: uint256 _amount,
604: bool _shouldQueue,
605: bytes[] memory _data
606: ) internal {
607: if (poolStatus != PoolStatus.OPEN) revert DepositsDisabled();
608:
609: uint256 toDeposit = _amount;
610:
611: if (totalQueued == 0) {
612: uint256 queuedWithdrawals = withdrawalPool.getTotalQueuedWithdrawals();
613: if (queuedWithdrawals != 0) {
614: uint256 toDepositIntoQueue = toDeposit <= queuedWithdrawals
615: ? toDeposit
616: : queuedWithdrawals;
617: withdrawalPool.deposit(toDepositIntoQueue);
618: toDeposit -= toDepositIntoQueue;
619: IERC20Upgradeable(address(stakingPool)).safeTransfer(_account, toDepositIntoQueue);
620: }
621:
622: if (toDeposit != 0) {
623: uint256 canDeposit = stakingPool.canDeposit();
624: if (canDeposit != 0) {
625: uint256 toDepositIntoPool = toDeposit <= canDeposit ? toDeposit : canDeposit;
626: stakingPool.deposit(_account, toDepositIntoPool, _data);
627: toDeposit -= toDepositIntoPool;
628: }
629: }
630: }
631:
632: if (toDeposit != 0) {
633: if (_shouldQueue) {
634: _requireNotPaused();
635: if (accountIndexes[_account] == 0) {
636: accounts.push(_account);
637: accountIndexes[_account] = accounts.length - 1;
638: }
639: accountQueuedTokens[_account] += toDeposit;
640: totalQueued += toDeposit;
641: } else {
642: token.safeTransfer(_account, toDeposit);
643: }
644: }
645:
646: emit Deposit(_account, _amount - toDeposit, _shouldQueue ? toDeposit : 0);
647: }
As can be seen, if the totalQueued is 0, it deposits tokens without checking the paused state between the Lines: 611-630. However, it only checks whether the pool is open which is not part of pausability and the variable can be false while the paused state is true or vice-versa.
withdrawalPool is not pausable either.
Here´s the withdraw function:
Contract: PriorityPool.sol
271: function withdraw(
272: uint256 _amountToWithdraw,
273: uint256 _amount,
274: uint256 _sharesAmount,
275: bytes32[] calldata _merkleProof,
276: bool _shouldUnqueue,
277: bool _shouldQueueWithdrawal
278: ) external {
279: if (_amountToWithdraw == 0) revert InvalidAmount();
280:
281: uint256 toWithdraw = _amountToWithdraw;
282: address account = msg.sender;
283:
284:
285: if (_shouldUnqueue == true) {
286: _requireNotPaused();
287:
288: if (_merkleProof.length != 0) {
289: bytes32 node = keccak256(
290: bytes.concat(keccak256(abi.encode(account, _amount, _sharesAmount)))
291: );
292: if (!MerkleProofUpgradeable.verify(_merkleProof, merkleRoot, node))
293: revert InvalidProof();
294: } else if (accountIndexes[account] < merkleTreeSize) {
295: revert InvalidProof();
296: }
297:
298: uint256 queuedTokens = getQueuedTokens(account, _amount);
299: uint256 canUnqueue = queuedTokens <= totalQueued ? queuedTokens : totalQueued;
300: uint256 amountToUnqueue = toWithdraw <= canUnqueue ? toWithdraw : canUnqueue;
301:
302: if (amountToUnqueue != 0) {
303: accountQueuedTokens[account] -= amountToUnqueue;
304: totalQueued -= amountToUnqueue;
305: toWithdraw -= amountToUnqueue;
306: emit UnqueueTokens(account, amountToUnqueue);
307: }
308: }
309:
310:
311: if (toWithdraw != 0) {
312: IERC20Upgradeable(address(stakingPool)).safeTransferFrom(
313: account,
314: address(this),
315: toWithdraw
316: );
317: toWithdraw = _withdraw(account, toWithdraw, _shouldQueueWithdrawal);
318: }
319:
320: token.safeTransfer(account, _amountToWithdraw - toWithdraw);
321: }
As can be seen too, if the user doesn´t want to unqueue their tokens(_shouldUnqueue = false), they can withdraw their stake without being entangled in the paused state as well.
Impact
During the paused state of this contract, it will continue to serve deposits and withdrawals
Tools Used
Manual Review
Recommendations
Use whenNotPaused modifier for both functions.
