Paladin - Warden Pledges contest - cccz's results

Lines of code

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

Vulnerability details

Impact

In the _pledge function, require delegationBoost.adjusted_balance_of(pledgeParams.receiver) + amount <= pledgeParams.targetVotes. In reality, when the user pledges amount votes, the actual votes received by the receiver are the bias in the following calculation. And the bias will be less than amount due to the loss of precision.

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

This means that the balance of receiver may not reach targetVotes

    point = self._checkpoint_read(_user, False)
    amount += (point.bias - point.slope * (block.timestamp - point.ts))
    return amount

Proof of Concept

https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L245-L246 https://github.com/curvefi/curve-veBoost/blob/master/contracts/BoostV2.vy#L192-L209 https://github.com/curvefi/curve-veBoost/blob/master/contracts/BoostV2.vy#L175

Tools Used

None

Recommended Mitigation Steps

Use bias instead of amount in the check below

        uint256 slope = amount / boostDuration;
        uint256 bias = slope * boostDuration;
        if(delegationBoost.adjusted_balance_of(pledgeParams.receiver) + bias > pledgeParams.targetVotes) revert Errors.TargetVotesOverflow();
        delegationBoost.boost(
            pledgeParams.receiver,
            amount,
            endTimestamp,
            user
        );

Lines of code

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

Vulnerability details

Impact

In the recoverERC20 function, when minAmountRewardToken[token] ! = 0, the token cannot be withdrawn by the owner. However, the owner can call the removeRewardToken function to make minAmountRewardToken[token] == 0 and then call the recoverERC20 function to rug that reward token.

Proof of Concept

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

Tools Used

None

Recommended Mitigation Steps

Consider removing the recoverERC20 function

Lines of code

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

Vulnerability details

Impact

When the system is paused, the creator cannot withdraw the remaining reward tokens via the retrievePledgeRewards/closePledge functions. Considering that when the system is paused, the user cannot increase the creator's vote via the _pledge function, the creator may want to withdraw his reward tokens since no new pledges are attracted, so the whenNotPaused modifier should not be added to the retrievePledgeRewards/closePledge functions. And if the system is paused indefinitely, the creator's reward tokens will be frozen in the contract

Proof of Concept

https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L456-L457 https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L488-L489

Tools Used

None

Recommended Mitigation Steps

Consider removing the whenNotPaused modifier from the retrievePledgeRewards/closePledge functions.

Lines of code

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

Vulnerability details

Impact

There are ERC20 tokens that charge fee for every transfer() or transferFrom().

The reward token may be fee-on-transfer token.

In the current implementation, createPledge/extendPledge/increasePledgeRewardPerVote assumes that the received amount is the same as the transfer amount, and uses it to calculate rewardPerVote.

As a result, in _pledge(), later users may not be able to successfully claim their rewards, as it may revert for insufficient balance. And in retrievePledgeRewards()/closePledge(), the creator cannot withdraw the remaining reward tokens due to insufficient balance.

Proof of Concept

https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L266-L271 https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L466-L475 https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L502-L508

Tools Used

None

Recommended Mitigation Steps

Consider getting the received amount by calculating the difference of token balance (using balanceOf) before and after the transferFrom.