Paladin - Warden Pledges contest - csanuragjain's results

Lines of code

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

Vulnerability details

Impact

The recoverERC20 function restricts owner from recovering reward tokens which are in use by contract. However the owner can simply bypass this by using removeRewardToken which will remove the Reward token from usage and allow admin to steal these funds

Proof of Concept

1. Assume multiple pledges are made using Token Y and currently minAmountRewardToken[token] is set to 10
2. Owner wants to steal the collected funds from pledges but is restricted by below condition in recoverERC20

function recoverERC20(address token) external onlyOwner returns(bool) {
        if(minAmountRewardToken[token] != 0) revert Errors.CannotRecoverToken();

       ...
    }

3. To bypass this, owner simply calls removeRewardToken with token Y address

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

3. This sets minAmountRewardToken[Y] = 0;

4. Now owner can simply call recoverERC20 function and steal away all token Y funds

Recommended Mitigation Steps

Place all owner functions behind a Timelock which will sufficient time to contract users to prevent their funds

Lines of code

https://github.com/code-423n4/2022-10-paladin/blob/main/contracts/WardenPledge.sol#L438 https://github.com/code-423n4/2022-10-paladin/blob/main/contracts/WardenPledge.sol#L333

Vulnerability details

Impact

The contract is not handling fee on transfer token properly. This can lead to disastrous consequences including stealing from other users pledge reward token, User unable to retrieve their reward tokens as shown in POC

Proof of Concept

1. User X starts a pledge on token T1 using createPledge function
2. Let's say totalRewardAmount is calculated to be 1000
3. User transfers the amount but since there is a fee on transfer (say 10%) so contract only receive amount 900
4. Since this contract is not handling fee on transfer token properly so accounting is done incorrectly where totalRewardAmount=1000 even though contract got 900 amount

vars.totalRewardAmount = (rewardPerVote * vars.votesDifference * vars.duration) / UNIT;
        vars.feeAmount = (vars.totalRewardAmount * protocalFeeRatio) / MAX_PCT ;
        if(vars.totalRewardAmount > maxTotalRewardAmount) revert Errors.IncorrectMaxTotalRewardAmount();
        if(vars.feeAmount > maxFeeAmount) revert Errors.IncorrectMaxFeeAmount();

        // Pull all the rewards in this contract
        IERC20(rewardToken).safeTransferFrom(creator, address(this), vars.totalRewardAmount);
        // And transfer the fees from the Pledge creator to the Chest contract
        IERC20(rewardToken).safeTransferFrom(creator, chestAddress, vars.feeAmount);

5. Now lets say another User Y starts a pledge on Token T1 using createPledge function
6. Let's say totalRewardAmount is calculated to be 2000 for this user and he transfers the same
7. Due to fee on transfer actual amount transferred is 1800
8. Finally contract has below state

Pledge User 1:
pledgeAvailableRewardAmounts[User X Pledge id]=1000

Pledge User 2:
pledgeAvailableRewardAmounts[User Y Pledge id]=2000

Contract Balance for Token T:
900+1800=2700

9. Now lets say user Y closes the pledge using closePledge
10. This transfers the pledgeAvailableRewardAmounts[User Y Pledge id] which is 2000 to User Y

IERC20(pledgeParams.rewardToken).safeTransfer(receiver, remainingAmount);

11. Now remaining balance left in contract for this token T is 2700-2000=700

12. User X tries to close pledge but it fails since contract does not have sufficient funds left

pledgeAvailableRewardAmounts[User X Pledge id]=1000

Contract Balance for Token T:700

Recommended Mitigation Steps

Calculate the contract balance before and after transfer to calculate the exact amount transferred to the contract. This way pledgeAvailableRewardAmounts will be updated correctly