search

code-423n4 / 2021-10-pooltogether-findings

0 stars 0 forks source link

`RingBufferLib.newestIndex` returns wrong value when no entries #27

Closed code423n4 closed 3 years ago

code423n4 commented 3 years ago

Handle

cmichel

Vulnerability details

The RingBufferLib.newestIndex returns _cardinality - 1 if _nextIndex = 0. This is correct if the buffer's capacity is at a maximum, but wrong if it has just been created.

Impact

The TwabLib.newestTwab function returns _twabs[MAX_CARDINALITY - 1]. As everything is uninitialized at this point, it should not lead to issues though but other functions using newestIndex could.

Recommended Mitigation Steps

Returning 0 in this case seems more sensible? Anyway, before accessing any buffer, it should be checked if it has any elements in it, otherwise, short-circuit with a default value.

asselstine commented 3 years ago

I think this is now out-of-date?

https://github.com/pooltogether/v4-core/blob/5357854748c5a673a9e4d30c285cf051f76bf103/contracts/libraries/RingBufferLib.sol#L44

GalloDaSballo commented 3 years ago

After looking at commit history, it seems like this finding was relevant during the preview, but not during the contest. Am setting to invalid