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

Vulnerability details

Description

WardenPledge contract has a sweeping function (recoverERC20) to handle mistakenly sent ERC20 tokens:

    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 code will not allow registered reward tokens to be transfered due to the rugging potential. It checks if minAmountRewardToken[token] != 0, which is True for registered tokens.

However, owner can easily bypass this check using removeRewardToken function:

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

This function sets any minAmountRewardToken[token] that is not 0, to 0.

Therefore, owner can instantly call:

1. removeRewardToken(TokenX)
2. recoverERC20(TokenX) to steal all the funds.

It’s very important to not allow owner such dangerous operation because:

1. It is not necessary for the functioning of the protocol
2. It hurts user’s trust of the protocol
3. Compromised owner can instantly steal all funds
4. Malicious owner can instantly steal all funds.

Impact

Owner can steal all ERC20 tokens including rewards, of the contract.

Tools Used

Manual audit

Recommended Mitigation Steps

This threat can be fully mitigated using these steps:

1. add mapping(IERC20 => bool) public disabledTokens; state member
2. In recoverERC20, require disabledTokens[token] == False
3. In removeRewardToken, after settings minAmountRewardToken to 0, add:
   disabledTokens[token] == True

This solution will limit only reward tokens from being claimed by the owner, which is the desired behavior.

---

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

All reactions