Lines of code
<https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L653-L661&gt;

Vulnerability details

Impact

Admin can rug all of the contract’s funds

Proof of Concept

The function recoverERC20() is only callable by the owner and its goal is: @notice Recovers ERC2O tokens sent by mistake to the contract. The call fails if minAmountRewardToken[token] != 0 , which is the check made to make sure that what’s being recovered isn’t a whitelisted token.

However, the following can be done in 1 transaction:

1. The owner calls removeRewardToken() to set minAmountRewardToken = 0 on any whitelisted token:

File: WardenPledge.sol
585:     function removeRewardToken(address token) external onlyOwner {
586:         if(token == address(0)) revert Errors.ZeroAddress();
587:         if(minAmountRewardToken[token] == 0) revert Errors.NotAllowedToken();
588:         
589:         minAmountRewardToken[token] = 0;
590:         
591:         emit RemoveRewardToken(token);
592:     }

2. The owner calls recoverERC20() on the un-whitelisted token to get the funds:

File: WardenPledge.sol
653:     function recoverERC20(address token) external onlyOwner returns(bool) {
654:         if(minAmountRewardToken[token] != 0) revert Errors.CannotRecoverToken();
655: 
656:         uint256 amount = IERC20(token).balanceOf(address(this));
657:         if(amount == 0) revert Errors.NullValue();
658:         IERC20(token).safeTransfer(owner(), amount);
659: 
660:         return true;
661:     }

Notice that a line such as this suggests that the contract does own whitelisted tokens:

File: WardenPledge.sol
332:         // Pull all the rewards in this contract
333:         IERC20(rewardToken).safeTransferFrom(creator, address(this), vars.totalRewardAmount);

3. The owner repeats 1) & 2) on all whitelisted tokens

4. The owner runs away

Recommended Mitigation Steps

The following balance check would prevent the above rug scenario but could be grieved by someone transferring even 1 token to the contract:

File: WardenPledge.sol
585:     function removeRewardToken(address token) external onlyOwner {
586:         if(token == address(0)) revert Errors.ZeroAddress();
587:         if(minAmountRewardToken[token] == 0) revert Errors.NotAllowedToken();
588:         
+ 588:         if(IERC20(token).balanceOf(address(this)) > 0) revert Errors.NotAllowedToken();
589:         minAmountRewardToken[token] = 0;
590:         
591:         emit RemoveRewardToken(token);
592:     }

Therefore, I believe the best solution would be putting in place an internal accounting and use the internal balance to check that a reward token can be removed.

---

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

All reactions