Lack of definite endTimestamp period for proxies

[hats-bug-reporter[bot]]

Github username: -- Twitter username: 97Sabit Submission hash (on-chain): 0xef1046a72d84382edfbf2e15abd9b886db3129210aee953ad1c16dc2a57ce72f Severity: low

Description: Description\ The setVoterProxy function currently allows proxies to be assigned without a definite endTimestamp specified. This allows proxies to have indefinite voting rights on behalf of a user.

Not having definite end timestamps for proxies undermines users' ability to control and revoke voting rights. It increases the risk of compromised or unauthorized proxies retaining power indefinitely.

As a result, system becomes more centralized as users cannot regain voting rights from proxies.

1. Proof of Concept (PoC) File

https://github.com/PaladinFinance/Vote-Flywheel/blob/cf3c82f102a76f58acf003980c480eb9028f0e94/contracts/LootVoteController.sol#L423

[PlamenTSV]

There is an end timestamp on the link you provided?

[ololade97]

There is an end timestamp on the link you provided?

I mean a period of time. end timestamp in the code is infinite.

[chainNue]

it's limited by userLockEnd

uint256 userLockEnd = IHolyPalPower(hPalPower).locked__end(user);
if(endTimestamp < block.timestamp || endTimestamp > userLockEnd) revert Errors.InvalidTimestamp();

[ololade97]

it's limited by userLockEnd

uint256 userLockEnd = IHolyPalPower(hPalPower).locked__end(user);
if(endTimestamp < block.timestamp || endTimestamp > userLockEnd) revert Errors.InvalidTimestamp();

I mean 50,000 years endtime could be set.

[Kogaroshi]

This is indeed invalid, as it is limited by the user's lock end timestamp, which can be at maximum 2 years (max lock duration on hPAL), so the proxy votes can at maximum allowed to the end of user lock. For them to be or not until the end of the Lock is up to them or the Proxy Managers they chose to allow on their lock. And finding #45 seems to be a duplicate of this report, just worded differently, but in the mechanism designated in both reports is the same (the endTimestamp parameter when setting up a proxy), so I will invalidate both for the same reason.

[ololade97]

This is indeed invalid, as it is limited by the user's lock end timestamp, which can be at maximum 2 years (max lock duration on hPAL), so the proxy votes can at maximum allowed to the end of user lock. For them to be or not until the end of the Lock is up to them or the Proxy Managers they chose to allow on their lock. And finding #45 seems to be a duplicate of this report, just worded differently, but in the mechanism designated in both reports is the same (the endTimestamp parameter when setting up a proxy), so I will invalidate both for the same reason.

45 is not a duplicate

[ololade97]

45 is about proxy manager setting himself as a proxy.

[Kogaroshi]

The fact that a proxy manager can set themselves as proxy is not an issue, there is no will to prevent from such scenario. An user can set a Proxy Manager to be allowed to set proxies on his voting power in his name, and if the Manager judges that the best uses is to have the proxy for himself and set up the gauge votes for this user (and potentially other ones), there is no issue with that type of scenario, hence why I didn't consider it as part of the issue in the report.

[ololade97]

The fact that a proxy manager can set themselves as proxy is not an issue, there is no will to prevent from such scenario. An user can set a Proxy Manager to be allowed to set proxies on his voting power in his name, and if the Manager judges that the best uses is to have the proxy for himself and set up the gauge votes for this user (and potentially other ones), there is no issue with that type of scenario, hence why I didn't consider it as part of the issue in the report.

Ok. However, I couldn't see where max lock duration is set to two years in hPal.

[Kogaroshi]

https://doc.paladin.vote/governance/holy-pal-hpal/smart-contract#constants

MAX_LOCK_DURATION uint256 public constant MAX_LOCK_DURATION = 63115200; // 2 years Maximum duration of a Lock

[ololade97]

Great. The 2 years seems to be missing in the code for audit. If it is not in the code, that means endTime is infinite.

[Kogaroshi]

It's not missing, the function you describe as faulty checks for the Lock end timestamp, from the HolyPalPower contract, in which you can see this end timestamp is calculated based on the user Lock start timestamp and the duration of the Lock, both fetched from the UserLock struct from the HolyPaladinToken. And in HolyPaladinToken we have the required checks to make sure that the duration is between 3 months and 2 years. There is no need to have that constant again in any other contracts, that would be redundant and an useless constant there. But there is no case the endTimestamp is infinite.

[ololade97]

Ok, thanks.