A single point of failure is not acceptable for this project

[code423n4]

Lines of code

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

Vulnerability details

Impact

The pause() function on WardenPledge.sol has a single point of failure and onlyOwner can stop all project.

Owner is not behind a multisig and changes are not behind a timelock.(This information hasnt got in documents)

Even if protocol admins/developers are not malicious there is still a chance for admin keys to be stolen.

   function pause() external onlyOwner {
        _pause();
    }

In addition, when we add other onlyOwner privileges, the single point of failure situation becomes stronger;

onlyOwner   |   Functions
========================
onlyOwner  =>  addMultipleRewardToken(address[],uint256[])
onlyOwner  =>  addRewardToken(address,uint256)
onlyOwner  =>  updateRewardToken(address,uint256)
onlyOwner  =>  removeRewardToken(address)
onlyOwner  =>  updateChest(address)
onlyOwner  =>  updateMinTargetVotes(uint256)
onlyOwner  =>  updatePlatformFee(uint256)
onlyOwner  =>  pause()
onlyOwner  =>  unpause()
onlyOwner  =>  recoverERC20(address)

See this example where a similar finding has been flagged as a high-severity issue:
realitycards-findings

Recommended Mitigation Steps

Isolate functionu and other lightweight onlyOwner functions such as pause() that are very powerful and will affect the project

Add a time lock to critical functions like pause()

Admin-only functions that change critical parameters should emit events and have timelocks. Events allow capturing the changed parameters so that off-chain tools/interfaces can register such changes with timelocks that allow users to evaluate them and consider if they would like to engage/exit based on how they perceive the changes as affecting the trustworthiness of the protocol or profitability of the implemented financial services.

Allow only multi-signature wallets to call the function to reduce the likelihood of an attack.

https://twitter.com/danielvf/status/1572963475101556738?s=20&t=V1kvzfJlsx-D2hfnG0OmuQ

[Kogaroshi]

This design where 1 address has the onlyOwner controlled by 1 address is by choice, so that Owner can be the Paladin Core Multisig, allowing to react quickly and prevent any loss of funds or exploit in the early months after deploy. This is to be later on moved to give the control to a Timelock once the Paladin governance moves on-chain.
And all admin functions do emit Event (either from the WardenPledge code, or from the inherited code)

[c4-judge]

kirk-baird marked the issue as duplicate of #70

[c4-judge]

kirk-baird marked the issue as satisfactory

[c4-judge]

Simon-Busch marked the issue as duplicate of #269