Lines of code

Vulnerability details

Impact

There is a recoverERC20() function to withdraw ERC20 tokens from the contract.

Currently, it checks if the token isn’t an active reward token but it can be passed easily if the admin removes the reward token using removeRewardToken().

So if the admin removes an active reward token and withdraws all amount of the token from the contract, the pledge creator using that token will lose their funds permanently.

Even if the admin doesn’t do that by fault, it’s a potential admin attack vector to steal all user funds.

Proof of Concept

• As we can see, recoverERC20() can withdraw the entire balance of the token if it’s not an active reward token.

  function recoverERC20(address token) external onlyOwner returns(bool) {
      if(minAmountRewardToken[token] != 0) revert Errors.CannotRecoverToken();
  
      uint256 amount = IERC20(token).balanceOf(address(this));
      if(amount == 0) revert Errors.NullValue();
      IERC20(token).safeTransfer(owner(), amount);
  
      return true;
  }
  

• The admin can remove tokens from reward token using removeRewardToken() even if there is an active pledge using this token.

  function removeRewardToken(address token) external onlyOwner {
      if(token == address(0)) revert Errors.ZeroAddress();
      if(minAmountRewardToken[token] == 0) revert Errors.NotAllowedToken();
      
      minAmountRewardToken[token] = 0;
      
      emit RemoveRewardToken(token);
  }
  

• So if the admin removes the reward token and calls recoverERC20() by fault or maliciously to steal funds, all pledge creators who used this token will lose their funds.

Tools Used

Manual Review

Recommended Mitigation Steps

I think we should add an additional mapping for the reward token like activePledgeCount to count the number of active pledges to use the token and update the mapping properly when users create/close the pledges.

And we can check if activePledgeCount[token] == 0 in the removeRewardToken() so that the admin can’t remove the reward token that has an active pledge.

    function removeRewardToken(address token) external onlyOwner {
        if(token == address(0)) revert Errors.ZeroAddress();
        if(minAmountRewardToken[token] == 0) revert Errors.NotAllowedToken();
        
        if(activePledgeCount[token] > 0) revert Errors.ActivePledgeExists(); //++++++++++++++++++++++++++++

        minAmountRewardToken[token] = 0;
        
        emit RemoveRewardToken(token);
    }  

The text was updated successfully, but these errors were encountered:

All reactions