Function getPastTotalLocks may return incorrect values

[code423n4]

Lines of code

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

Vulnerability details

Impact

totalLocks array can contain elements with the same fromBlock property but different total locked amounts. This edge case is not considered in the implementation of getPastTotalLock function, which returns a value as soon as it finds a TotalLock with matching fromBlock property (see HolyPaladinToken.sol:520-522). If there are more total locks with the same block number that function should return the latest one, i.e. the one with the highest index.
In its current implementation getPastTotalLock function can also return a different value after changing the length of the log, either from the correct value to an incorrect or the other way.

Recommended mitigation steps

There are two obvious ways of fixing this issue: either by removing the check in HolyPaladinToken.sol:520-522)) or by not allowing adding elements to the totalLocks array with the same fromBlock properties. While both ways are viable, the latter one is preferred as it is a gas optimization.
Checks should be added whether or not the block number of the last element of totalLocks array is equal to the current block number, and if it is, modify that element instead of adding a new one.

[Kogaroshi]

Duplicate of #20
See in this issue for PR with changes

[0xean]

closing as same warden submitted #86 with the same vulnerability.