code423n4 commented 2 years ago

Lines of code

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

Vulnerability details

Impact

Part of the stake can be withdrawn immediately at any time.

Proof of Concept

Ratio between COOLDOWN_PERIOD and UNSTAKE_PERIOD = 2 only and should be much higher (ex 5 or 10)

l.28 COOLDOWN_PERIOD = 864000; // 10 days ... UNSTAKE_PERIOD = 432000; // 5 days

Various algorithms exist that would allow users to optimize their withdrawal tactics. By using such tactics, stakers may be able to withdraw at least a part of the stake immediately.

Let’s assume that the values are the same as in tests: COOLDOWN_PERIOD == 1 hour and UNSTAKE_PERIOD == 30 minutes. Stakers can split their stake into 3 parts and call cooldown for one of them every 30 minutes. That would ensure that at least 1⁄3 of the stake can be withdrawn immediately at any time. And on average, more than 1⁄2 of the stake can be withdrawn immediately.

Remediation: Make sure that the COOLDOWN_SECONDS value is much larger than the UNSTAKE_WINDOW. This will make any cooldown optimization techniques less effective.

as seen in 'Aave Safety Module' Consensys Diligence Audit: https://consensys.net/diligence/audits/2020/09/aave-safety-module/#unhandled-return-values-of-transfer-and-transferfrom

l.1107 // Inspired by stkAAVE cooldown system function _getNewReceiverCooldown(...

original function: https://github.com/aave/safety-module/blob/3c8d5c30302c35239b6ea8c23ad3df36b485d7b6/contracts/stake/StakedToken.sol#L238