Paladin - Warden Pledges contest - Jeiwan's results

Lines of code

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

Vulnerability details

Impact

Owner can steal pending pledge rewards at any time. Even though there's an attempt to restrict this owner's ability, it can be easily bypassed.

Proof of Concept

The recoverERC20 function allows owner to withdraw ERC20 tokens mistakenly sent to the contract (WardenPledge.sol#L653):

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

Since pledge rewards are ERC20 tokens sent to the contract (WardenPledge.sol#L333), owner can withdraw them at any time. This check can be easily bypassed by owner:

if(minAmountRewardToken[token] != 0) revert Errors.CannotRecoverToken();

Because minAmountRewardToken is controlled by owner (WardenPledge.sol#L570-L592):

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

Exploit Scenario

Owner calls removeRewardToken for each of the currently supported reward tokens, which will set minAmountRewardToken to 0 for them. Owner calls recoverERC20 for each of the tokens and removes all pending pledge reward from the contract.

Tools Used

Manual review

Recommended Mitigation Steps

Consider disallowing owner transferring any ERC20 tokens out of the contract. Alternatively, in the recoverERC20 function, ensure no open pledge has remaining reward in the token being recovered.

Lines of code

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

Vulnerability details

[M-05] Pledge target is not realistically reachable

Targets

• https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L245

Impact

A pledge target can be fully reached only when a pledge is made every block, which is not realistic on Ethereum. Every pledge will have some amount of leftover reward tokens and target vote amounts won't be fully maintained during desired durations.

Proof of Concept

A pledge target consists of two parameters:

1. number of votes;
2. duration.

Thus, the total reward of a pledge is calculated as reward per token * number of votes desired * duration (WardenPledge.sol#L327):

vars.votesDifference = targetVotes - votingEscrow.balanceOf(receiver);
vars.totalRewardAmount = (rewardPerVote * vars.votesDifference * vars.duration) / UNIT;

Thus, pledge creator expects to have a desired amount of votes maintained during the entire duration. However, it's almost not possible to maintain a specific number of votes during a specific duration for two reasons:

1. Delegated votes decrease each second. This is how BoostV2 works (BoostV2.vy#L99-L111), and WardenPledge also takes this into account during pledging (WardenPledge.sol#L259-L263).
2. At any given second, no more than the target pledge votes can be delegated to a pledge receiver (WardenPledge.sol#L245).

Thus, a pledge target can only be reached at the current timestamp, but, in the next block, the amount of delegated votes will decrease and a new pledge will be required to fill the gap.

Tools Used

Manual review

Recommended Mitigation Steps

The only reasonable workaround for this problem seems to be setting a higher target, which means overpaying. And it still doesn't allow to maintain a desired amount of votes during a desired duration.

Lines of code

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

Vulnerability details

Impact

Total reward amount in extendPledge and increasePledgeRewardPerVote can be calculated incorrectly due to cached pledgeParams.votesDifference, which can lead to two outcomes:

1. total reward amount is higher, thus a portion of it won't be claimable;
2. total reward amount is lower, thus the pledge target won't be reached.

Proof of Concept

When a pledge is created, the creator chooses the target–the total amount of votes they want to reach with the pledge. Based on a target, the number of missing votes is calculated, which is then used to calculated the total reward amount (WardenPledge.sol#L325-L327):

function createPledge(
    address receiver,
    address rewardToken,
    uint256 targetVotes,
    uint256 rewardPerVote, // reward/veToken/second
    uint256 endTimestamp,
    uint256 maxTotalRewardAmount,
    uint256 maxFeeAmount
) external whenNotPaused nonReentrant returns(uint256){
    ...
    // Get the missing votes for the given receiver to reach the target votes
    // We ignore any delegated boost here because they might expire during the Pledge duration
    // (we can have a future version of this contract using adjusted_balance)
    vars.votesDifference = targetVotes - votingEscrow.balanceOf(receiver);

    vars.totalRewardAmount = (rewardPerVote * vars.votesDifference * vars.duration) / UNIT;
    ...
  }

When extending a pledge or increasing a pledge reward per vote, current veToken balance of the pledge's receiver (votingEscrow.balanceOf(receiver)) can be different from the one it had when the pledge was created (e.g. the receiver managed to lock more CRV or some of locked tokens have expired). However pledgeParams.votesDifference is not recalculated (WardenPledge.sol#L387, WardenPledge.sol#L432):

function extendPledge(
    uint256 pledgeId,
    uint256 newEndTimestamp,
    uint256 maxTotalRewardAmount,
    uint256 maxFeeAmount
) external whenNotPaused nonReentrant {
    ...
    Pledge storage pledgeParams = pledges[pledgeId];
    ...
    uint256 totalRewardAmount = (pledgeParams.rewardPerVote * pledgeParams.votesDifference * addedDuration) / UNIT;
    ...
}

function increasePledgeRewardPerVote(
    uint256 pledgeId,
    uint256 newRewardPerVote,
    uint256 maxTotalRewardAmount,
    uint256 maxFeeAmount
) external whenNotPaused nonReentrant {
    ...
    Pledge storage pledgeParams = pledges[pledgeId];
    ...
    uint256 totalRewardAmount = (rewardPerVoteDiff * pledgeParams.votesDifference * remainingDuration) / UNIT;
    ...
}

This can lead to two consequences:

1. When receiver's veToken balance has increased (i.e. votesDifference got in fact smaller), pledge creator will overpay for pledge extension and pledge reward per vote increase. This extra reward cannot be received by pledgers because a receiver cannot get more votes than pledgeParams.targetVotes (which is not updated when modifying a pledge):

   function _pledge(uint256 pledgeId, address user, uint256 amount, uint256 endTimestamp) internal {
       ...
       // Check that this will not go over the Pledge target of votes
       if(delegationBoost.adjusted_balance_of(pledgeParams.receiver) + amount > pledgeParams.targetVotes) revert Errors.TargetVotesOverflow();
       ...
   }

2. When receiver's veToken balance has decreased (i.e. votesDifference got in fact bigger), the pledge target cannot be reached because the reward amount was underpaid in extendPledge/increasePledgeRewardPerVote.

Tools Used

Manual review

Recommended Mitigation Steps

Consider updating votesDifference when extending a pledge or increasing a pledge reward per vote.

[N-01] Pledged event is emitted with a wrong amount of delegated votes

Targets

• https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L273

Proof of Concept

The actual amount of delegated votes is bias, not amount (WardenPledge.sol#L255-L263):

uint256 slope = amount / boostDuration;
uint256 bias = slope * boostDuration;

// Rewards are set in the Pledge as reward/veToken/sec
// To find the total amount of veToken delegated through the whole Boost duration
// based on the Boost bias & the Boost duration, to take in account that the delegated amount decreases
// each second of the Boost duration
uint256 totalDelegatedAmount = ((bias * boostDuration) + bias) / 2;

Tools Used

Manual review

Recommended Mitigation Steps

Consider using bias instead of amount.