QA Report

[code423n4]

Lines of code

https://github.com/code-423n4/2022-03-paladin/blob/main/contracts/HolyPaladinToken.sol#L394

Vulnerability details

Impact

If there is not enough PAL in the referenced rewardsVault or hPAL contract isn't added as a spender to the rewardsVault, the claim() function will be failing, i.e. user reward funds be locked until hPAL approval and funds transfer done manually.

Setting severity to medium as rewards claiming is core contract function and its availability is impacted

Proof of Concept

claim() tries to move funds from rewardsVault to requested user, while there were no approvals or spender addition done in the contract:

https://github.com/code-423n4/2022-03-paladin/blob/main/contracts/HolyPaladinToken.sol#L394

PaladinRewardReserve need spender contract to be added so enable funds access:

https://github.com/code-423n4/2022-03-paladin/blob/main/contracts/PaladinRewardReserve.sol#L28-L34

Recommended Mitigation Steps

Consider calling setNewSpender in the hPAL constructor:

https://github.com/code-423n4/2022-03-paladin/blob/main/contracts/HolyPaladinToken.sol#L194

Also, consider adding a transfer of initial PAL funds to rewardsVault to constructor/initialization phase

[Kogaroshi]

This behavior is necessary, since the PaladinRewardReserve might be used for other contract that will need to distribute rewards. Hence the hPAL contract cannot be the admin of the Reward Reserve. See other PaladinRewardReserve relative Issue to see changes applied to the contract

And for the previous amount of funds, it is the team role to ensure this step is not missed while deploying the smart contracts

[0xean]

I am going to downgrade this to QA. If this step was somehow missed it would still be possible to do it once discovered without risk to user's funds.

[JeeberC4]

Judge downgraded to QA, warden did not submit a QA Report, preserving original title: Rewards vault setup isn't ensured before interactions