stake.link
stake.link
DeFiHardhatBridge
27,500 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

A malicious Lock owner can cause DOS of protocol using out-of-bound index error

angrymustacheman

Summary

Due to lack of checks for numUpdates variable , a malicious lock owner can cause DOS of protocol by using out-of-bounds error in SDLPoolSecondary.sol#_executeQueuedLockUpdates .

Vulnerability Details

Let's understand the vulnerability. Checkout this executeQueuedOperations function at :

https://github.com/Cyfrin/2023-12-stake-link/blob/main/contracts/core/sdlPool/SDLPoolSecondary.sol#L247C1-L250C6

function executeQueuedOperations(uint256[] memory _lockIds) external {
_executeQueuedLockUpdates(msg.sender, _lockIds);
_mintQueuedNewLocks(msg.sender);
}

it can be called by any User as it doesn't have a modifier. It then calls _executeQueuedLockUpdates function which calls a _onlyLockOwner(lockId, _owner); function to prevents call from anyone other than Lock Owner.

https://github.com/Cyfrin/2023-12-stake-link/blob/main/contracts/core/sdlPool/SDLPoolSecondary.sol#L456C13-L456C44

...
_onlyLockOwner(lockId, _owner);
...

But if we look closely into this function, we see numUpdates value is never checked . As we know there can be cases where queuedLockUpdates[lockId].length could be 0 . In such cases where queuedLockUpdates[lockId].length=numUpdates=0 and j=0(as in the first iteration) , we might skip the while loop to directly enter the k-variable-for-loop where numUpdates=j=0 and we know executing a pop() on zero length array would cause unexpected DOS condition like out-of-bound error for protocol. :

https://github.com/Cyfrin/2023-12-stake-link/blob/main/contracts/core/sdlPool/SDLPoolSecondary.sol#L451C2-L510C6

function _executeQueuedLockUpdates(address _owner, uint256[] memory _lockIds) internal updateRewards(_owner) {
uint256 finalizedBatchIndex = _getFinalizedUpdateBatchIndex();
​
for (uint256 i = 0; i < _lockIds.length; ++i) {
uint256 lockId = _lockIds[i];
_onlyLockOwner(lockId, _owner);
@> uint256 numUpdates = queuedLockUpdates[lockId].length;
​
Lock memory curLockState = locks[lockId];
@> uint256 j = 0;
while (j < numUpdates) { <@ skips this as j=numUpdates
if (queuedLockUpdates[lockId][j].updateBatchIndex > finalizedBatchIndex) break;
​
Lock memory updateLockState = queuedLockUpdates[lockId][j].lock;
int256 baseAmountDiff = int256(updateLockState.amount) - int256(curLockState.amount);
int256 boostAmountDiff = int256(updateLockState.boostAmount) - int256(curLockState.boostAmount);
​
if (baseAmountDiff < 0) {
emit Withdraw(_owner, lockId, uint256(-1 * baseAmountDiff));
if (updateLockState.amount == 0) {
delete locks[lockId];
delete lockOwners[lockId];
balances[_owner] -= 1;
if (tokenApprovals[lockId] != address(0)) delete tokenApprovals[lockId];
emit Transfer(_owner, address(0), lockId);
} else {
locks[lockId].amount = updateLockState.amount;
}
sdlToken.safeTransfer(_owner, uint256(-1 * baseAmountDiff));
} else if (boostAmountDiff < 0) {
locks[lockId].expiry = updateLockState.expiry;
locks[lockId].boostAmount = 0;
emit InitiateUnlock(_owner, lockId, updateLockState.expiry);
} else {
locks[lockId] = updateLockState;
uint256 totalDiff = uint256(baseAmountDiff + boostAmountDiff);
effectiveBalances[_owner] += totalDiff;
totalEffectiveBalance += totalDiff;
emit UpdateLock(
_owner,
lockId,
updateLockState.amount,
updateLockState.boostAmount,
updateLockState.duration
);
}
curLockState = updateLockState;
++j;
}
​
for (uint256 k = 0; k < numUpdates; ++k) {
@> if (j == numUpdates) {
DOS here @> queuedLockUpdates[lockId].pop();
} else {
queuedLockUpdates[lockId][k] = queuedLockUpdates[lockId][j];
++j;
}
}
}
}

Impact

Malicious Lock Owner can cause DOS of protocol by using out-of-bounds error for multiple times

Tools Used

Manual Review

Recommendations

Add this check before while loop :

require(numUpdates!=0,"numUpdates should be non-zero");
Updates

Lead Judging Commences

0kage Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.