Paladin - Warden Pledges contest - Aymen0909's results

Lines of code

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

Vulnerability details

Impact

The recoverERC20 function allows the owner to withdraw the ERC20 tokens sent by acceident to the contract but it doesn't allow him to withdraw pldged tokens, the owner though could use the removeRewardToken function to remove a token used currently in a pledge and then he can use recoverERC20 function to drain all it's balance, and thus the owner could drain any existing pledge balance with this technique.

Proof of Concept

The recoverERC20 function below contains a check to prevent the owner from withdrawing tokens used in a pledge :

function recoverERC20(address token) external onlyOwner returns(bool) {
    // @audit check to prevent the withdrawal of pledge tokens 
    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;
}

But using the removeRewardToken function the owner can eliminate the check and thus he will be able to withdraw all the balance of the pledge token.

function removeRewardToken(address token) external onlyOwner {
    if(token == address(0)) revert Errors.ZeroAddress();
    if(minAmountRewardToken[token] == 0) revert Errors.NotAllowedToken();
    
    // @audit this will invalidate the check in recoverERC20
    minAmountRewardToken[token] = 0;
    
    emit RemoveRewardToken(token);
}

Tools Used

Manual review

Recommended Mitigation Steps

To avoid this issue the the contract must use more than just minAmountRewardToken to track pledged tokens, i recommend to use a Token struct containing the minAmountRewardToken and a boolean variable to check if the token is used in a pledge or not, this boolean value should be checked before removing any token.

Lines of code

https://github.com/code-423n4/2022-10-paladin/blob/main/contracts/WardenPledge.sol#L387-L388 https://github.com/code-423n4/2022-10-paladin/blob/main/contracts/WardenPledge.sol#L432-L433

Vulnerability details

Impact

When the pledge creator tries to extend his pledge or to increase the rewardPerVote of the pledge the old votesDifference set when creating the pledge is used to calculate the totalRewardAmount and the feeAmount, this will force the creator to pay a higher fee to the contract even though the votesDifference of the pledge has changed.

Proof of Concept

In the extendPledge and increasePledgeRewardPerVote functions the initial votes difference pledgeParams.votesDifference is used to calculate the fee amount as shown in the code below :

uint256 totalRewardAmount = (pledgeParams.rewardPerVote * pledgeParams.votesDifference * addedDuration) / UNIT;
uint256 feeAmount = (totalRewardAmount * protocalFeeRatio) / MAX_PCT ;

uint256 totalRewardAmount = (rewardPerVoteDiff * pledgeParams.votesDifference * remainingDuration) / UNIT;
uint256 feeAmount = (totalRewardAmount * protocalFeeRatio) / MAX_PCT ;

The scenario where the pledge creator will be forced to pay a higher fee without reason is the following :

• The creator sets the initial votes target equal to 1000 for example (suppose that the creator has 0 votes initially).

• At the time the creator decides to extend the pledge (or increase the rewerPerToken) he has already collected 800 votes, so only 200 votes are needed to achieve the target meaning that votesDifference == 200.

• When the contract tries to calculate the totalRewardAmount and the feeAmount it will use the votesDifference == 1000 instead votesDifference == 200 and so the creator will pay more in feeAmount but he will not be able to get more than 1000 votes because of the check :

if(delegationBoost.adjusted_balance_of(pledgeParams.receiver) + amount > pledgeParams.targetVotes) revert Errors.TargetVotesOverflow();

Tools Used

Manual review

Recommended Mitigation Steps

To avoid this issue both the extendPledge and increasePledgeRewardPerVote functions should calculate and update the actual votes difference using the code below :.

pledgeParams.votesDifference = targetVotes - votingEscrow.balanceOf(receiver);

In this way the creator will pay a fee only for the remaining votes in the pledge.