Platform: Code4rena
Start Date: 30/04/2024
Pot Size: $112,500 USDC
Total HM: 22
Participants: 122
Period: 8 days
Judge: alcueca
Total Solo HM: 1
Id: 372
League: ETH
Rank: 49/122
Findings: 1
Award: $18.20
🌟 Selected for report: 0
🚀 Solo Findings: 0
18.1958 USDC - $18.20
In the removeCollateralToken() function of RestakeManager contract, any token can be removed by the admin of the contract.
function removeCollateralToken( IERC20 _collateralTokenToRemove ) external onlyRestakeManagerAdmin { // Remove it from the list uint256 tokenLength = collateralTokens.length; for (uint256 i = 0; i < tokenLength; ) { if (address(collateralTokens[i]) == address(_collateralTokenToRemove)) { collateralTokens[i] = collateralTokens[collateralTokens.length - 1]; collateralTokens.pop(); emit CollateralTokenRemoved(_collateralTokenToRemove); return; } unchecked { ++i; } } // If the item was not found, throw an error revert NotFound(); }
The issue with the function is that there is no check if there is any undone withdrawals related to the token being removed, which potentially will cause users lose their redeem amount.
Bob calls withdraw function and initiates a withdraw.Because of the coolDownPeriod, he has to wait for the claim. In this period, the admin of RestakeManager contract, removes the collateralToken. After enough time has passed, Bob calls claim() function but he will not get the any tokens due to the removal.
function claim(uint256 withdrawRequestIndex) external nonReentrant { // check if provided withdrawRequest Index is valid if (withdrawRequestIndex >= withdrawRequests[msg.sender].length) revert InvalidWithdrawIndex(); WithdrawRequest memory _withdrawRequest = withdrawRequests[msg.sender][ withdrawRequestIndex ]; if (block.timestamp - _withdrawRequest.createdAt < coolDownPeriod) revert EarlyClaim(); // subtract value from claim reserve for claim asset claimReserve[_withdrawRequest.collateralToken] -= _withdrawRequest.amountToRedeem; // delete the withdraw request withdrawRequests[msg.sender][withdrawRequestIndex] = withdrawRequests[msg.sender][ withdrawRequests[msg.sender].length - 1 ]; withdrawRequests[msg.sender].pop(); // burn ezETH locked for withdraw request ezETH.burn(address(this), _withdrawRequest.ezETHLocked); // send selected redeem asset to user if (_withdrawRequest.collateralToken == IS_NATIVE) { payable(msg.sender).transfer(_withdrawRequest.amountToRedeem); } else { IERC20(_withdrawRequest.collateralToken).transfer( msg.sender, _withdrawRequest.amountToRedeem );//@audit if the token is removed by the admin, this wont work } // emit the event emit WithdrawRequestClaimed(_withdrawRequest); }
Manual Review
Before removing any asset,implement necessary check for the token being removed.
function removeCollateralToken( IERC20 _collateralTokenToRemove ) external onlyRestakeManagerAdmin { ADD THIS=> if(claimReserve[_collateralTokenToRemove] != 0) revert CanNotBeRemoved(); // Remove it from the list uint256 tokenLength = collateralTokens.length; for (uint256 i = 0; i < tokenLength; ) { if (address(collateralTokens[i]) == address(_collateralTokenToRemove)) { collateralTokens[i] = collateralTokens[collateralTokens.length - 1]; collateralTokens.pop(); emit CollateralTokenRemoved(_collateralTokenToRemove); return; } unchecked { ++i; } } // If the item was not found, throw an error revert NotFound(); }
Other
#0 - alcueca
2024-05-17T14:03:08Z
Copy of #271?
#1 - c4-judge
2024-05-17T14:05:47Z
alcueca marked the issue as unsatisfactory: Invalid
#2 - c4-judge
2024-05-20T04:34:14Z
alcueca changed the severity to 2 (Med Risk)
#3 - c4-judge
2024-05-20T04:41:25Z
alcueca marked the issue as satisfactory
18.1958 USDC - $18.20
In the removeCollateralToken() function of RestakeManager contract, any token can be removed by the admin of the contract.
function removeCollateralToken( IERC20 _collateralTokenToRemove ) external onlyRestakeManagerAdmin { // Remove it from the list uint256 tokenLength = collateralTokens.length; for (uint256 i = 0; i < tokenLength; ) { if (address(collateralTokens[i]) == address(_collateralTokenToRemove)) { collateralTokens[i] = collateralTokens[collateralTokens.length - 1]; collateralTokens.pop(); emit CollateralTokenRemoved(_collateralTokenToRemove); return; } unchecked { ++i; } } // If the item was not found, throw an error revert NotFound(); }
The issue with the function is that there is no check if there is any undone withdrawals related to the token being removed, which potentially will cause users lose their redeem amount.
Bob calls withdraw function and initiates a withdraw.Because of the coolDownPeriod, he has to wait for the claim. In this period, the admin of RestakeManager contract, removes the collateralToken. After enough time has passed, Bob calls claim() function but he will not get the any tokens due to the removal.
function claim(uint256 withdrawRequestIndex) external nonReentrant { // check if provided withdrawRequest Index is valid if (withdrawRequestIndex >= withdrawRequests[msg.sender].length) revert InvalidWithdrawIndex(); WithdrawRequest memory _withdrawRequest = withdrawRequests[msg.sender][ withdrawRequestIndex ]; if (block.timestamp - _withdrawRequest.createdAt < coolDownPeriod) revert EarlyClaim(); // subtract value from claim reserve for claim asset claimReserve[_withdrawRequest.collateralToken] -= _withdrawRequest.amountToRedeem; // delete the withdraw request withdrawRequests[msg.sender][withdrawRequestIndex] = withdrawRequests[msg.sender][ withdrawRequests[msg.sender].length - 1 ]; withdrawRequests[msg.sender].pop(); // burn ezETH locked for withdraw request ezETH.burn(address(this), _withdrawRequest.ezETHLocked); // send selected redeem asset to user if (_withdrawRequest.collateralToken == IS_NATIVE) { payable(msg.sender).transfer(_withdrawRequest.amountToRedeem); } else { IERC20(_withdrawRequest.collateralToken).transfer( msg.sender, _withdrawRequest.amountToRedeem );//@audit if the token is removed by the admin, this wont work } // emit the event emit WithdrawRequestClaimed(_withdrawRequest); }
Manual Review
Before removing any asset,implement necessary check for the token being removed.
function removeCollateralToken( IERC20 _collateralTokenToRemove ) external onlyRestakeManagerAdmin { ADD THIS=> if(claimReserve[_collateralTokenToRemove] != 0) revert CanNotBeRemoved(); // Remove it from the list uint256 tokenLength = collateralTokens.length; for (uint256 i = 0; i < tokenLength; ) { if (address(collateralTokens[i]) == address(_collateralTokenToRemove)) { collateralTokens[i] = collateralTokens[collateralTokens.length - 1]; collateralTokens.pop(); emit CollateralTokenRemoved(_collateralTokenToRemove); return; } unchecked { ++i; } } // If the item was not found, throw an error revert NotFound(); }
Other
#0 - alcueca
2024-05-17T14:01:46Z
Copy of #271?
#1 - c4-judge
2024-05-17T14:01:58Z
alcueca marked the issue as not a duplicate
#2 - c4-judge
2024-05-17T14:02:04Z
alcueca marked the issue as duplicate of #271
#3 - c4-judge
2024-05-17T14:04:30Z
alcueca marked the issue as duplicate of #97
#4 - c4-judge
2024-05-17T14:05:46Z
alcueca marked the issue as unsatisfactory: Invalid
#5 - c4-judge
2024-05-20T04:34:14Z
alcueca changed the severity to 2 (Med Risk)
#6 - c4-judge
2024-05-20T04:41:21Z
alcueca marked the issue as satisfactory
18.1958 USDC - $18.20
In the removeCollateralToken() function of RestakeManager contract, any token can be removed by the admin of the contract.
function removeCollateralToken( IERC20 _collateralTokenToRemove ) external onlyRestakeManagerAdmin { // Remove it from the list uint256 tokenLength = collateralTokens.length; for (uint256 i = 0; i < tokenLength; ) { if (address(collateralTokens[i]) == address(_collateralTokenToRemove)) { collateralTokens[i] = collateralTokens[collateralTokens.length - 1]; collateralTokens.pop(); emit CollateralTokenRemoved(_collateralTokenToRemove); return; } unchecked { ++i; } } // If the item was not found, throw an error revert NotFound(); }
The issue with the function is that there is no check if there is any undone withdrawals related to the token being removed, which potentially will cause users lose their redeem amount.
Bob calls withdraw function and initiates a withdraw.Because of the coolDownPeriod, he has to wait for the claim. In this period, the admin of RestakeManager contract, removes the collateralToken. After enough time has passed, Bob calls claim() function but he will not get the any tokens due to the removal.
function claim(uint256 withdrawRequestIndex) external nonReentrant { // check if provided withdrawRequest Index is valid if (withdrawRequestIndex >= withdrawRequests[msg.sender].length) revert InvalidWithdrawIndex(); WithdrawRequest memory _withdrawRequest = withdrawRequests[msg.sender][ withdrawRequestIndex ]; if (block.timestamp - _withdrawRequest.createdAt < coolDownPeriod) revert EarlyClaim(); // subtract value from claim reserve for claim asset claimReserve[_withdrawRequest.collateralToken] -= _withdrawRequest.amountToRedeem; // delete the withdraw request withdrawRequests[msg.sender][withdrawRequestIndex] = withdrawRequests[msg.sender][ withdrawRequests[msg.sender].length - 1 ]; withdrawRequests[msg.sender].pop(); // burn ezETH locked for withdraw request ezETH.burn(address(this), _withdrawRequest.ezETHLocked); // send selected redeem asset to user if (_withdrawRequest.collateralToken == IS_NATIVE) { payable(msg.sender).transfer(_withdrawRequest.amountToRedeem); } else { IERC20(_withdrawRequest.collateralToken).transfer( msg.sender, _withdrawRequest.amountToRedeem );//@audit if the token is removed by the admin, this wont work } // emit the event emit WithdrawRequestClaimed(_withdrawRequest); }
Manual Review
Before removing any asset,implement necessary check for the token being removed.
function removeCollateralToken( IERC20 _collateralTokenToRemove ) external onlyRestakeManagerAdmin { ADD THIS=> if(claimReserve[_collateralTokenToRemove] != 0) revert CanNotBeRemoved(); // Remove it from the list uint256 tokenLength = collateralTokens.length; for (uint256 i = 0; i < tokenLength; ) { if (address(collateralTokens[i]) == address(_collateralTokenToRemove)) { collateralTokens[i] = collateralTokens[collateralTokens.length - 1]; collateralTokens.pop(); emit CollateralTokenRemoved(_collateralTokenToRemove); return; } unchecked { ++i; } } // If the item was not found, throw an error revert NotFound(); }
Other
#0 - c4-judge
2024-05-17T14:00:53Z
alcueca marked the issue as not a duplicate
#1 - c4-judge
2024-05-17T14:01:02Z
alcueca changed the severity to 2 (Med Risk)
#2 - c4-judge
2024-05-17T14:01:18Z
alcueca changed the severity to 3 (High Risk)
#3 - c4-judge
2024-05-17T14:01:32Z
alcueca marked the issue as duplicate of #271
#4 - alcueca
2024-05-17T14:01:39Z
Copy of #271?
#5 - c4-judge
2024-05-17T14:04:31Z
alcueca marked the issue as duplicate of #97
#6 - c4-judge
2024-05-17T14:05:46Z
alcueca marked the issue as unsatisfactory: Invalid
#7 - c4-judge
2024-05-20T04:34:15Z
alcueca changed the severity to 2 (Med Risk)
#8 - c4-judge
2024-05-20T04:41:21Z
alcueca marked the issue as satisfactory
18.1958 USDC - $18.20
In the removeCollateralToken() function of RestakeManager contract, any token can be removed by the admin of the contract.
function removeCollateralToken( IERC20 _collateralTokenToRemove ) external onlyRestakeManagerAdmin { // Remove it from the list uint256 tokenLength = collateralTokens.length; for (uint256 i = 0; i < tokenLength; ) { if (address(collateralTokens[i]) == address(_collateralTokenToRemove)) { collateralTokens[i] = collateralTokens[collateralTokens.length - 1]; collateralTokens.pop(); emit CollateralTokenRemoved(_collateralTokenToRemove); return; } unchecked { ++i; } } // If the item was not found, throw an error revert NotFound(); }
The issue with the function is that there is no check if there is any undone withdrawals related to the token being removed, which potentially will cause users lose their redeem amount.
Bob calls withdraw function and initiates a withdraw.Because of the coolDownPeriod, he has to wait for the claim. In this period, the admin of RestakeManager contract, removes the collateralToken. After enough time has passed, Bob calls claim() function but he will not get the any tokens due to the removal.
function claim(uint256 withdrawRequestIndex) external nonReentrant { // check if provided withdrawRequest Index is valid if (withdrawRequestIndex >= withdrawRequests[msg.sender].length) revert InvalidWithdrawIndex(); WithdrawRequest memory _withdrawRequest = withdrawRequests[msg.sender][ withdrawRequestIndex ]; if (block.timestamp - _withdrawRequest.createdAt < coolDownPeriod) revert EarlyClaim(); // subtract value from claim reserve for claim asset claimReserve[_withdrawRequest.collateralToken] -= _withdrawRequest.amountToRedeem; // delete the withdraw request withdrawRequests[msg.sender][withdrawRequestIndex] = withdrawRequests[msg.sender][ withdrawRequests[msg.sender].length - 1 ]; withdrawRequests[msg.sender].pop(); // burn ezETH locked for withdraw request ezETH.burn(address(this), _withdrawRequest.ezETHLocked); // send selected redeem asset to user if (_withdrawRequest.collateralToken == IS_NATIVE) { payable(msg.sender).transfer(_withdrawRequest.amountToRedeem); } else { IERC20(_withdrawRequest.collateralToken).transfer( msg.sender, _withdrawRequest.amountToRedeem );//@audit if the token is removed by the admin, this wont work } // emit the event emit WithdrawRequestClaimed(_withdrawRequest); }
Manual Review
Before removing any asset,implement necessary check for the token being removed.
function removeCollateralToken( IERC20 _collateralTokenToRemove ) external onlyRestakeManagerAdmin { ADD THIS=> if(claimReserve[_collateralTokenToRemove] != 0) revert CanNotBeRemoved(); // Remove it from the list uint256 tokenLength = collateralTokens.length; for (uint256 i = 0; i < tokenLength; ) { if (address(collateralTokens[i]) == address(_collateralTokenToRemove)) { collateralTokens[i] = collateralTokens[collateralTokens.length - 1]; collateralTokens.pop(); emit CollateralTokenRemoved(_collateralTokenToRemove); return; } unchecked { ++i; } } // If the item was not found, throw an error revert NotFound(); }
Other
#0 - c4-judge
2024-05-17T13:58:09Z
alcueca marked the issue as not a duplicate
#1 - c4-judge
2024-05-17T13:58:38Z
alcueca changed the severity to 2 (Med Risk)
#2 - c4-judge
2024-05-17T13:58:54Z
alcueca marked the issue as primary issue
#3 - c4-judge
2024-05-17T14:03:49Z
alcueca changed the severity to 3 (High Risk)
#4 - c4-judge
2024-05-17T14:04:34Z
alcueca marked the issue as duplicate of #97
#5 - c4-judge
2024-05-17T14:05:45Z
alcueca marked the issue as unsatisfactory: Invalid
#6 - c4-judge
2024-05-20T04:34:15Z
alcueca changed the severity to 2 (Med Risk)
#7 - c4-judge
2024-05-20T04:41:18Z
alcueca marked the issue as satisfactory