code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-03-paladin/blob/9c26ec8556298fb1dc3cf71f471aadad3a5c74a0/contracts/HolyPaladinToken.sol#L692-L694

Vulnerability details

The README.md states:

If the user has a Lock, and delegates to someone, then the bonus voting power is not counted.

https://github.com/code-423n4/2022-03-paladin/blob/9c26ec8556298fb1dc3cf71f471aadad3a5c74a0/README.md?plain=1#L100

Impact

Accounts are still able to claim bonus voting power even if they delegate to someone else, and any operations that rely on the public functions getPastVotes() and getPastDelegate() for the hPAL holder account, as long as they first delegate to another account, then in a new block, delegate to themselves and then to the account to which they ultimately want to delegate. This chain of operations may or may not be intentional, leading to incorrect vote results.

Proof of Concept

Calling delegate() always adds a new entry to the delegateCheckpoints[delegator] array, regardless of the block.number:

    function _delegate(address delegator, address delegatee) internal {
        // Move delegation from the old delegate to the given delegate
        address oldDelegatee = delegates[delegator];
        uint256 delegatorBalance = balanceOf(delegator);
        delegates[delegator] = delegatee;

        // update the the Delegate chekpoint for the delegatee
        delegateCheckpoints[delegator].push(DelegateCheckpoint(safe32(block.number), delegatee));

https://github.com/code-423n4/2022-03-paladin/blob/9c26ec8556298fb1dc3cf71f471aadad3a5c74a0/contracts/HolyPaladinToken.sol#L1317-L1324

Calling delegate() to the hPAL holder in a new block ensures that it is the first entry in the array with that block.number. Calling delegate() in the same block but to another account causes a new entry to be added, but with the same block.number.

The code that looks up whether an account has been delegated to, only checks that the block number of the entry in the array matches, and does not consider multiple entries with the same block number:

            if (delegateCheckpoints[account][mid].fromBlock == blockNumber) {
                return delegateCheckpoints[account][mid].delegate;
            }

https://github.com/code-423n4/2022-03-paladin/blob/9c26ec8556298fb1dc3cf71f471aadad3a5c74a0/contracts/HolyPaladinToken.sol#L692-L694