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

Vulnerability details

Impact

There is no guarantee that the owner of the WardenPledge contract does not become compromised or malicious in the future. If this owner becomes compromised or malicious, after a pledge is created and the corresponding reward token amount is deposited, such owner can call the removeRewardToken function to remove the reward token from the whitelist first and then call the recoverERC20 function to receive the reward token amount deposited for the created pledge. As a result, such owner successfully steals the reward token amount deposited by the pledge creator.

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

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

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

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

Proof of Concept

Please append the following test in the pause & unpause describe block in test\wardenPledge.test.ts. This test will pass to demonstrate the described scenario.

        it.only("Compromised or malicious owner of WardenPledge contract can steal pledge creator's deposited reward token amount", async () => {
            const rewardToken1BalanceAdminBefore = await rewardToken1.balanceOf(admin.address)
            const rewardToken1BalanceCreatorBefore = await rewardToken1.balanceOf(creator.address)

            // creator calls the createPledge function
            await wardenPledge.connect(creator).createPledge(
                receiver.address,
                rewardToken1.address,
                target_votes,
                reward_per_vote,
                end_timestamp,
                max_total_reward_amount,
                max_fee_amount
            )

            // if admin, who is the owner of the wardenPledge contract, becomes compromised or malicious,
            //   admin can first remove rewardToken1 from the whitelist
            //   and then call the recoverERC20 function to receive all of creator's deposited rewardToken1 amount
            await wardenPledge.connect(admin).removeRewardToken(rewardToken1.address)
            await wardenPledge.connect(admin).recoverERC20(rewardToken1.address)

            // as a result, admin gains creator's deposited rewardToken1 amount
            const rewardToken1BalanceAdminAfter = await rewardToken1.balanceOf(admin.address)
            expect(rewardToken1BalanceAdminAfter).to.be.gt(rewardToken1BalanceAdminBefore)

            // creator loses the deposited rewardToken1 amount
            const rewardToken1BalanceCreatorAfter = await rewardToken1.balanceOf(creator.address)
            expect(rewardToken1BalanceCreatorAfter).to.be.lt(rewardToken1BalanceCreatorBefore)
        });

Tools Used

VSCode

Recommended Mitigation Steps

The recoverERC20 function can be updated to include an additional to address input and be only callable by a governance contract so only the governance can decide on whether to send the relevant token amount from the WardenPledge contract to the to address.

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

All reactions