code-423n4 / 2022-03-paladin-findings

0 stars 0 forks source link

updating the state #27

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Lines of code

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

Vulnerability details

Impact

In the Emergency withdraw function userCurrentBonusRatio and durationRatio aren't update which will user clime funds with the wrong ratio

Proof of Concept

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

Tools Used

Manual

Recommended Mitigation Steps

set these variables to zero in the EmergencyWithdraw function

Kogaroshi commented 2 years ago

Issue already raised in https://github.com/code-423n4/2022-03-paladin-findings/issues/14 PR with the changes: https://github.com/PaladinFinance/Paladin-Tokenomics/pull/5

contesting the severity of the issue: even after an emergency withdraw, where the BonusRatio of the user isn't reset back to 0, users have no way to claim extra accrued rewards, as the claim method will be block by the Emergency state: https://github.com/code-423n4/2022-03-paladin/blob/9c26ec8556298fb1dc3cf71f471aadad3a5c74a0/contracts/HolyPaladinToken.sol#L381

0xean commented 2 years ago

Given that the contract can be moved back out of the emergency state I don't think the sponsors assessment of this being a low risk issue is correct. I do think due to the external circumstances required for this to be achieved that it is probably best qualifies as a medium risk

2 — Med: Assets not at direct risk, but the function of the protocol or its availability could be impacted, or leak value with a hypothetical attack path with stated assumptions, but external requirements.

Will mark #14 a duplicate of this issue as well.