Malicious owner can steal reward tokens

[code423n4]

Lines of code

https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L652-L661

Vulnerability details

The recoverERC20 function allows the contract owner to transfer arbitrary ERC20 tokens owned by the WardenPledge contract in order to recover tokens sent by mistake to the contract. In order to protect against withdrawal of deposited reward tokens, it includes a check that the withdrawn token is not currently an approved rewards token:

recoverERC20

/**
 * @notice Recovers ERC2O tokens sent by mistake to the contract
 * @dev Recovers ERC2O tokens sent by mistake to the contract
 * @param token Address tof the EC2O token
 * @return bool: success
 */
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;
}

However, the contract owner also has the ability to remove rewards tokens, and can easily bypass the check in recoverERC20 by first calling removeRewardToken to set minAmountRewardToken[token] to zero, then calling recoverERC20:

removeRewardToken

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);
}

Impact: A malicious owner can steal all deposited rewards tokens.

Recommendation: Disallow removing rewards tokens associated with active pledges. This will probably require tracking this information in a separately stored data structure.

[Kogaroshi]

Duplicate of #17

[c4-judge]

kirk-baird marked the issue as satisfactory

[c4-judge]

kirk-baird marked the issue as not a duplicate

[c4-judge]

kirk-baird marked the issue as duplicate

[c4-judge]

Simon-Busch marked the issue as duplicate of #68