code-423n4 / 2021-05-fairside-findings

0 stars 0 forks source link

`TributeAccrual` missing out-of-bounds checks #46

Closed code423n4 closed 3 years ago

code423n4 commented 3 years ago

Handle

cmichel

Vulnerability details

Vulnerability Details

The _addTribute and _addGovernanceTribute functions underflow when there are no tributes:

Tribute storage lastTribute = tributes[totalTributes - 1] = tributes[-1]; // underflow

Impact

It's bad practice and the iteration with the offset in totalAvailableTribute will miss the initial tribute unless called with strange parameters that overflow themselves.

Recommended Mitigation Steps

Add a special case totalTributes (totalGovernanceTributes) == 0.

fairside-core commented 3 years ago

No out of bounds check is necessary in this instance. The underflow at the first iteration will cause the "last" entry in the mapping to be read which will be completely 0. This will cause the else branch to execute that submits a new entry properly at the 0 index. I do not see how totalAvailableTribute will miss the initial tribute.

cemozerr commented 3 years ago

Labeling this issue as invalid, as @fairside-core's correct that the else branch will execute.